From f1e6dce047d091bc4b4ea9264a29401f39381fa6 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Thu, 21 Nov 2013 16:19:19 -0700 Subject: [PATCH 01/23] Update test scripts to always run ALL tests, even when some fail --- hack/make/dyntest | 39 ++++++++++++++++++++++++++++----------- hack/make/test | 37 +++++++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/hack/make/dyntest b/hack/make/dyntest index ff607d391..756b03820 100644 --- a/hack/make/dyntest +++ b/hack/make/dyntest @@ -19,18 +19,35 @@ fi bundle_test() { { date - for test_dir in $(find_test_dirs); do ( - set -x - cd $test_dir + + TESTS_FAILED=() + for test_dir in $(find_test_dirs); do + echo - # Install packages that are dependencies of the tests. - # Note: Does not run the tests. - go test -i -ldflags "$LDFLAGS" $BUILDFLAGS - - # Run the tests with the optional $TESTFLAGS. - export TEST_DOCKERINIT_PATH=$DEST/../dynbinary/dockerinit-$VERSION - go test -v -ldflags "$LDFLAGS -X github.com/dotcloud/docker/utils.INITSHA1 \"$DOCKER_INITSHA1\"" $BUILDFLAGS $TESTFLAGS - ) done + if ! ( + set -x + cd $test_dir + + # Install packages that are dependencies of the tests. + # Note: Does not run the tests. + go test -i -ldflags "$LDFLAGS" $BUILDFLAGS + + # Run the tests with the optional $TESTFLAGS. + export TEST_DOCKERINIT_PATH=$DEST/../dynbinary/dockerinit-$VERSION + go test -v -ldflags "$LDFLAGS -X github.com/dotcloud/docker/utils.INITSHA1 \"$DOCKER_INITSHA1\"" $BUILDFLAGS $TESTFLAGS + ); then + TESTS_FAILED+=("$test_dir") + sleep 1 # give it a second, so observers watching can take note + fi + done + + # if some tests fail, we want the bundlescript to fail, but we want to + # try running ALL the tests first, hence TESTS_FAILED + if [ "${#TESTS_FAILED[@]}" -gt 0 ]; then + echo + echo "Test failures in: ${TESTS_FAILED[@]}" + false + fi } 2>&1 | tee $DEST/test.log } diff --git a/hack/make/test b/hack/make/test index 45ffe87fb..361f731d7 100644 --- a/hack/make/test +++ b/hack/make/test @@ -13,17 +13,34 @@ set -e bundle_test() { { date - for test_dir in $(find_test_dirs); do ( - set -x - cd $test_dir + + TESTS_FAILED=() + for test_dir in $(find_test_dirs); do + echo - # Install packages that are dependencies of the tests. - # Note: Does not run the tests. - go test -i -ldflags "$LDFLAGS $LDFLAGS_STATIC" $BUILDFLAGS - - # Run the tests with the optional $TESTFLAGS. - go test -v -ldflags "$LDFLAGS $LDFLAGS_STATIC" $BUILDFLAGS $TESTFLAGS - ) done + if ! ( + set -x + cd $test_dir + + # Install packages that are dependencies of the tests. + # Note: Does not run the tests. + go test -i -ldflags "$LDFLAGS $LDFLAGS_STATIC" $BUILDFLAGS + + # Run the tests with the optional $TESTFLAGS. + go test -v -ldflags "$LDFLAGS $LDFLAGS_STATIC" $BUILDFLAGS $TESTFLAGS + ); then + TESTS_FAILED+=("$test_dir") + sleep 1 # give it a second, so observers watching can take note + fi + done + + # if some tests fail, we want the bundlescript to fail, but we want to + # try running ALL the tests first, hence TESTS_FAILED + if [ "${#TESTS_FAILED[@]}" -gt 0 ]; then + echo + echo "Test failures in: ${TESTS_FAILED[@]}" + false + fi } 2>&1 | tee $DEST/test.log } From 96b5be9dd9606a9bfbf0fdbe98bcaf8b6e77e4b1 Mon Sep 17 00:00:00 2001 From: Sven Dowideit Date: Thu, 14 Nov 2013 15:19:31 +1000 Subject: [PATCH 02/23] add more searchable info to the error message when ADD tries to go outside the context --- buildfile.go | 2 +- integration/buildfile_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/buildfile.go b/buildfile.go index ce157302f..b643af509 100644 --- a/buildfile.go +++ b/buildfile.go @@ -288,7 +288,7 @@ func (b *buildFile) addContext(container *Container, orig, dest string) error { destPath = destPath + "/" } if !strings.HasPrefix(origPath, b.context) { - return fmt.Errorf("Forbidden path: %s", origPath) + return fmt.Errorf("Forbidden path outside the build context: %s (%s)", orig, origPath) } fi, err := os.Stat(origPath) if err != nil { diff --git a/integration/buildfile_test.go b/integration/buildfile_test.go index 964b58403..20d0450a7 100644 --- a/integration/buildfile_test.go +++ b/integration/buildfile_test.go @@ -483,7 +483,7 @@ func TestForbiddenContextPath(t *testing.T) { t.Fail() } - if err.Error() != "Forbidden path: /" { + if err.Error() != "Forbidden path outside the build context: ../../ (/)" { t.Logf("Error message is not expected: %s", err.Error()) t.Fail() } From f16c45f8b0447249074a0b41807a979a3ae326c5 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Sun, 24 Nov 2013 20:00:39 -0700 Subject: [PATCH 03/23] Add space-escaping to path parts of lxc.mount.entry lines in generated lxc.conf, allowing for spaces in mount point names Fixes #2802 --- lxc_template.go | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/lxc_template.go b/lxc_template.go index 2ba286742..2c68b7a83 100644 --- a/lxc_template.go +++ b/lxc_template.go @@ -1,6 +1,7 @@ package docker import ( + "strings" "text/template" ) @@ -31,8 +32,8 @@ lxc.rootfs = {{$ROOTFS}} {{if and .HostnamePath .HostsPath}} # enable domain name support -lxc.mount.entry = {{.HostnamePath}} {{$ROOTFS}}/etc/hostname none bind,ro 0 0 -lxc.mount.entry = {{.HostsPath}} {{$ROOTFS}}/etc/hosts none bind,ro 0 0 +lxc.mount.entry = {{escapeFstabSpaces .HostnamePath}} {{escapeFstabSpaces $ROOTFS}}/etc/hostname none bind,ro 0 0 +lxc.mount.entry = {{escapeFstabSpaces .HostsPath}} {{escapeFstabSpaces $ROOTFS}}/etc/hosts none bind,ro 0 0 {{end}} # use a dedicated pts for the container (and limit the number of pseudo terminal @@ -84,27 +85,27 @@ lxc.cgroup.devices.allow = c 10:200 rwm lxc.pivotdir = lxc_putold # WARNING: procfs is a known attack vector and should probably be disabled # if your userspace allows it. eg. see http://blog.zx2c4.com/749 -lxc.mount.entry = proc {{$ROOTFS}}/proc proc nosuid,nodev,noexec 0 0 +lxc.mount.entry = proc {{escapeFstabSpaces $ROOTFS}}/proc proc nosuid,nodev,noexec 0 0 # WARNING: sysfs is a known attack vector and should probably be disabled # if your userspace allows it. eg. see http://bit.ly/T9CkqJ -lxc.mount.entry = sysfs {{$ROOTFS}}/sys sysfs nosuid,nodev,noexec 0 0 -lxc.mount.entry = devpts {{$ROOTFS}}/dev/pts devpts newinstance,ptmxmode=0666,nosuid,noexec 0 0 -#lxc.mount.entry = varrun {{$ROOTFS}}/var/run tmpfs mode=755,size=4096k,nosuid,nodev,noexec 0 0 -#lxc.mount.entry = varlock {{$ROOTFS}}/var/lock tmpfs size=1024k,nosuid,nodev,noexec 0 0 -lxc.mount.entry = shm {{$ROOTFS}}/dev/shm tmpfs size=65536k,nosuid,nodev,noexec 0 0 +lxc.mount.entry = sysfs {{escapeFstabSpaces $ROOTFS}}/sys sysfs nosuid,nodev,noexec 0 0 +lxc.mount.entry = devpts {{escapeFstabSpaces $ROOTFS}}/dev/pts devpts newinstance,ptmxmode=0666,nosuid,noexec 0 0 +#lxc.mount.entry = varrun {{escapeFstabSpaces $ROOTFS}}/var/run tmpfs mode=755,size=4096k,nosuid,nodev,noexec 0 0 +#lxc.mount.entry = varlock {{escapeFstabSpaces $ROOTFS}}/var/lock tmpfs size=1024k,nosuid,nodev,noexec 0 0 +lxc.mount.entry = shm {{escapeFstabSpaces $ROOTFS}}/dev/shm tmpfs size=65536k,nosuid,nodev,noexec 0 0 # Inject dockerinit -lxc.mount.entry = {{.SysInitPath}} {{$ROOTFS}}/.dockerinit none bind,ro 0 0 +lxc.mount.entry = {{escapeFstabSpaces .SysInitPath}} {{escapeFstabSpaces $ROOTFS}}/.dockerinit none bind,ro 0 0 # Inject env -lxc.mount.entry = {{.EnvConfigPath}} {{$ROOTFS}}/.dockerenv none bind,ro 0 0 +lxc.mount.entry = {{escapeFstabSpaces .EnvConfigPath}} {{escapeFstabSpaces $ROOTFS}}/.dockerenv none bind,ro 0 0 # In order to get a working DNS environment, mount bind (ro) the host's /etc/resolv.conf into the container -lxc.mount.entry = {{.ResolvConfPath}} {{$ROOTFS}}/etc/resolv.conf none bind,ro 0 0 +lxc.mount.entry = {{escapeFstabSpaces .ResolvConfPath}} {{escapeFstabSpaces $ROOTFS}}/etc/resolv.conf none bind,ro 0 0 {{if .Volumes}} {{ $rw := .VolumesRW }} {{range $virtualPath, $realPath := .Volumes}} -lxc.mount.entry = {{$realPath}} {{$ROOTFS}}/{{$virtualPath}} none bind,{{ if index $rw $virtualPath }}rw{{else}}ro{{end}} 0 0 +lxc.mount.entry = {{escapeFstabSpaces $realPath}} {{escapeFstabSpaces $ROOTFS}}/{{escapeFstabSpaces $virtualPath}} none bind,{{ if index $rw $virtualPath }}rw{{else}}ro{{end}} 0 0 {{end}} {{end}} @@ -144,6 +145,12 @@ lxc.cgroup.cpu.shares = {{.Config.CpuShares}} var LxcTemplateCompiled *template.Template +// Escape spaces in strings according to the fstab documentation, which is the +// format for "lxc.mount.entry" lines in lxc.conf. See also "man 5 fstab". +func escapeFstabSpaces(field string) string { + return strings.Replace(field, " ", "\\040", -1) +} + func getMemorySwap(config *Config) int64 { // By default, MemorySwap is set to twice the size of RAM. // If you want to omit MemorySwap, set it to `-1'. @@ -167,6 +174,7 @@ func init() { "getMemorySwap": getMemorySwap, "getHostConfig": getHostConfig, "getCapabilities": getCapabilities, + "escapeFstabSpaces": escapeFstabSpaces, } LxcTemplateCompiled, err = template.New("lxc").Funcs(funcMap).Parse(LxcTemplate) if err != nil { From b702edadb7d63feb1f40ebfae27a4d745d4b733e Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Sun, 24 Nov 2013 20:02:06 -0700 Subject: [PATCH 04/23] Format lxc_template.go with gofmt --- lxc_template.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lxc_template.go b/lxc_template.go index 2c68b7a83..2d95a2971 100644 --- a/lxc_template.go +++ b/lxc_template.go @@ -171,9 +171,9 @@ func getCapabilities(container *Container) *Capabilities { func init() { var err error funcMap := template.FuncMap{ - "getMemorySwap": getMemorySwap, - "getHostConfig": getHostConfig, - "getCapabilities": getCapabilities, + "getMemorySwap": getMemorySwap, + "getHostConfig": getHostConfig, + "getCapabilities": getCapabilities, "escapeFstabSpaces": escapeFstabSpaces, } LxcTemplateCompiled, err = template.New("lxc").Funcs(funcMap).Parse(LxcTemplate) From 1af6ffb9bb5a1dec665a56b1bdf621fd2e2b7377 Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Tue, 26 Nov 2013 00:07:59 -0700 Subject: [PATCH 05/23] Add explicit test strings for new escapeFstabSpaces function --- lxc_template_unit_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lxc_template_unit_test.go b/lxc_template_unit_test.go index ce5af1d32..ccdfec889 100644 --- a/lxc_template_unit_test.go +++ b/lxc_template_unit_test.go @@ -100,3 +100,21 @@ func grepFile(t *testing.T, path string, pattern string) { } t.Fatalf("grepFile: pattern \"%s\" not found in \"%s\"", pattern, path) } + +func TestEscapeFstabSpaces(t *testing.T) { + var testInputs = map[string]string{ + " ": "\\040", + "": "", + "/double space": "/double\\040\\040space", + "/some long test string": "/some\\040long\\040test\\040string", + "/var/lib/docker": "/var/lib/docker", + " leading": "\\040leading", + "trailing ": "trailing\\040", + } + for in, exp := range testInputs { + if out := escapeFstabSpaces(in); exp != out { + t.Logf("Expected %s got %s", exp, out) + t.Fail() + } + } +} From 82674372946b431f93add255e137f82822eecfc1 Mon Sep 17 00:00:00 2001 From: Yurii Rashkovskii Date: Tue, 26 Nov 2013 06:54:46 -0800 Subject: [PATCH 06/23] Fix command line help for docker save `docker save IMAGE DESTINATION` is not what `docker save` expects --- commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands.go b/commands.go index d992db2e6..f298e41d1 100644 --- a/commands.go +++ b/commands.go @@ -2195,7 +2195,7 @@ func (cli *DockerCli) CmdCp(args ...string) error { } func (cli *DockerCli) CmdSave(args ...string) error { - cmd := cli.Subcmd("save", "IMAGE DESTINATION", "Save an image to a tar archive") + cmd := cli.Subcmd("save", "IMAGE", "Save an image to a tar archive (streamed to stdout)") if err := cmd.Parse(args); err != nil { return err } From 8398abf0dcda8fa7818b69355af9b99ab4e8bdfc Mon Sep 17 00:00:00 2001 From: Tianon Gravi Date: Tue, 26 Nov 2013 09:36:46 -0700 Subject: [PATCH 07/23] Fix CHANGELOG: we ended up not merging the btrfs driver for last night's release --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75dd094bd..bdd91eadd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,8 @@ #### Notable features since 0.6.0 -* Storage drivers: choose from aufs, device mapper, vfs or btrfs. -* Standard Linux support: docker now runs on unmodified linux kernels and all major distributions. +* Storage drivers: choose from aufs, device-mapper, or vfs. +* Standard Linux support: docker now runs on unmodified Linux kernels and all major distributions. * Links: compose complex software stacks by connecting containers to each other. * Container naming: organize your containers by giving them memorable names. * Advanced port redirects: specify port redirects per interface, or keep sensitive ports private. From c707c587c1fbd8e68f1e9a3c64ee0aca764461eb Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 26 Nov 2013 20:16:16 +0000 Subject: [PATCH 08/23] Add ParseRun unit tests --- commands_unit_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 commands_unit_test.go diff --git a/commands_unit_test.go b/commands_unit_test.go new file mode 100644 index 000000000..b7e6e894b --- /dev/null +++ b/commands_unit_test.go @@ -0,0 +1,78 @@ +package docker + +import ( + "strings" + "testing" +) + +func parse(t *testing.T, args string) (*Config, *HostConfig, error) { + config, hostConfig, _, err := ParseRun(strings.Split(args+" ubuntu bash", " "), nil) + return config, hostConfig, err +} + +func mustParse(t *testing.T, args string) (*Config, *HostConfig) { + config, hostConfig, err := parse(t, args) + if err != nil { + t.Fatal(err) + } + return config, hostConfig +} + +func TestParseRunLinks(t *testing.T) { + if _, hostConfig := mustParse(t, "-link a:b"); len(hostConfig.Links) == 0 || hostConfig.Links[0] != "a:b" { + t.Fatalf("Error parsing links. Expected []string{\"a:b\"}, received: %v", hostConfig.Links) + } + if _, hostConfig := mustParse(t, "-link a:b -link c:d"); len(hostConfig.Links) < 2 || hostConfig.Links[0] != "a:b" || hostConfig.Links[1] != "c:d" { + t.Fatalf("Error parsing links. Expected []string{\"a:b\", \"c:d\"}, received: %v", hostConfig.Links) + } + if _, hostConfig := mustParse(t, ""); len(hostConfig.Links) != 0 { + t.Fatalf("Error parsing links. No link expected, received: %v", hostConfig.Links) + } + + if _, _, err := parse(t, "-link a"); err == nil { + t.Fatalf("Error parsing links. `-link a` should be an error but is not") + } + if _, _, err := parse(t, "-link"); err == nil { + t.Fatalf("Error parsing links. `-link` should be an error but is not") + } +} + +func TestParseRunAttach(t *testing.T) { + if config, _ := mustParse(t, "-a stdin"); !config.AttachStdin || config.AttachStdout || config.AttachStderr { + t.Fatalf("Error parsing attach flags. Expect only Stdin enabled. Received: in: %v, out: %v, err: %v", config.AttachStdin, config.AttachStdout, config.AttachStderr) + } + if config, _ := mustParse(t, "-a stdin -a stdout"); !config.AttachStdin || !config.AttachStdout || config.AttachStderr { + t.Fatalf("Error parsing attach flags. Expect only Stdin and Stdout enabled. Received: in: %v, out: %v, err: %v", config.AttachStdin, config.AttachStdout, config.AttachStderr) + } + if config, _ := mustParse(t, "-a stdin -a stdout -a stderr"); !config.AttachStdin || !config.AttachStdout || !config.AttachStderr { + t.Fatalf("Error parsing attach flags. Expect all attach enabled. Received: in: %v, out: %v, err: %v", config.AttachStdin, config.AttachStdout, config.AttachStderr) + } + if config, _ := mustParse(t, ""); config.AttachStdin || !config.AttachStdout || !config.AttachStderr { + t.Fatalf("Error parsing attach flags. Expect Stdin disabled. Received: in: %v, out: %v, err: %v", config.AttachStdin, config.AttachStdout, config.AttachStderr) + } + + if _, _, err := parse(t, "-a"); err == nil { + t.Fatalf("Error parsing attach flags, `-a` should be an error but is not") + } + if _, _, err := parse(t, "-a invalid"); err == nil { + t.Fatalf("Error parsing attach flags, `-a invalid` should be an error but is not") + } + if _, _, err := parse(t, "-a invalid -a stdout"); err == nil { + t.Fatalf("Error parsing attach flags, `-a stdout -a invalid` should be an error but is not") + } + if _, _, err := parse(t, "-a stdout -a stderr -d"); err == nil { + t.Fatalf("Error parsing attach flags, `-a stdout -a stderr -d` should be an error but is not") + } + if _, _, err := parse(t, "-a stdin -d"); err == nil { + t.Fatalf("Error parsing attach flags, `-a stdin -d` should be an error but is not") + } + if _, _, err := parse(t, "-a stdout -d"); err == nil { + t.Fatalf("Error parsing attach flags, `-a stdout -d` should be an error but is not") + } + if _, _, err := parse(t, "-a stderr -d"); err == nil { + t.Fatalf("Error parsing attach flags, `-a stderr -d` should be an error but is not") + } + if _, _, err := parse(t, "-d -rm"); err == nil { + t.Fatalf("Error parsing attach flags, `-d -rm` should be an error but is not") + } +} From c7661f40b6c6a23e5fe2090a94bd9fa6521242d7 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 26 Nov 2013 23:00:44 +0000 Subject: [PATCH 09/23] Make volumes opts more strict --- commands.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/commands.go b/commands.go index d992db2e6..db752447f 100644 --- a/commands.go +++ b/commands.go @@ -1666,8 +1666,11 @@ func (opts PathOpts) String() string { return fmt.Sprintf("%v", map[string]struc func (opts PathOpts) Set(val string) error { var containerPath string - splited := strings.SplitN(val, ":", 2) - if len(splited) == 1 { + if strings.Count(val, ":") > 2 { + return fmt.Errorf("bad format for volumes: %s", val) + } + + if splited := strings.SplitN(val, ":", 2); len(splited) == 1 { containerPath = splited[0] val = filepath.Clean(splited[0]) } else { @@ -1680,6 +1683,7 @@ func (opts PathOpts) Set(val string) error { return fmt.Errorf("%s is not an absolute path", containerPath) } opts[val] = struct{}{} + return nil } From 462e30dcbdc5010abe21b206bcd4be0c8bc71be6 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 26 Nov 2013 23:03:50 +0000 Subject: [PATCH 10/23] Add parseRun volume unit tests --- commands_unit_test.go | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/commands_unit_test.go b/commands_unit_test.go index b7e6e894b..2eac5ce60 100644 --- a/commands_unit_test.go +++ b/commands_unit_test.go @@ -76,3 +76,82 @@ func TestParseRunAttach(t *testing.T) { t.Fatalf("Error parsing attach flags, `-d -rm` should be an error but is not") } } + +func TestParseRunVolumes(t *testing.T) { + if config, hostConfig := mustParse(t, "-v /tmp"); hostConfig.Binds != nil { + t.Fatalf("Error parsing volume flags, `-v /tmp` should not mount-bind anything. Received %v", hostConfig.Binds) + } else if _, exists := config.Volumes["/tmp"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /tmp` is missing from volumes. Received %v", config.Volumes) + } + + if config, hostConfig := mustParse(t, "-v /tmp -v /var"); hostConfig.Binds != nil { + t.Fatalf("Error parsing volume flags, `-v /tmp -v /var` should not mount-bind anything. Received %v", hostConfig.Binds) + } else if _, exists := config.Volumes["/tmp"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /tmp` is missing from volumes. Recevied %v", config.Volumes) + } else if _, exists := config.Volumes["/var"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /var` is missing from volumes. Received %v", config.Volumes) + } + + if config, hostConfig := mustParse(t, "-v /hostTmp:/containerTmp"); hostConfig.Binds == nil || hostConfig.Binds[0] != "/hostTmp:/containerTmp" { + t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp` should mount-bind /hostTmp into /containeTmp. Received %v", hostConfig.Binds) + } else if _, exists := config.Volumes["/containerTmp"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /tmp` is missing from volumes. Received %v", config.Volumes) + } + + if config, hostConfig := mustParse(t, "-v /hostTmp:/containerTmp -v /hostVar:/containerVar"); hostConfig.Binds == nil || hostConfig.Binds[0] != "/hostTmp:/containerTmp" || hostConfig.Binds[1] != "/hostVar:/containerVar" { + t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp -v /hostVar:/containerVar` should mount-bind /hostTmp into /containeTmp and /hostVar into /hostContainer. Received %v", hostConfig.Binds) + } else if _, exists := config.Volumes["/containerTmp"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /containerTmp` is missing from volumes. Received %v", config.Volumes) + } else if _, exists := config.Volumes["/containerVar"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /containerVar` is missing from volumes. Received %v", config.Volumes) + } + + if config, hostConfig := mustParse(t, "-v /hostTmp:/containerTmp:ro -v /hostVar:/containerVar:rw"); hostConfig.Binds == nil || hostConfig.Binds[0] != "/hostTmp:/containerTmp:ro" || hostConfig.Binds[1] != "/hostVar:/containerVar:rw" { + t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp:ro -v /hostVar:/containerVar:rw` should mount-bind /hostTmp into /containeTmp and /hostVar into /hostContainer. Received %v", hostConfig.Binds) + } else if _, exists := config.Volumes["/containerTmp"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /containerTmp` is missing from volumes. Received %v", config.Volumes) + } else if _, exists := config.Volumes["/containerVar"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /containerVar` is missing from volumes. Received %v", config.Volumes) + } + + if config, hostConfig := mustParse(t, "-v /hostTmp:/containerTmp -v /containerVar"); hostConfig.Binds == nil || len(hostConfig.Binds) > 1 || hostConfig.Binds[0] != "/hostTmp:/containerTmp" { + t.Fatalf("Error parsing volume flags, `-v /hostTmp:/containerTmp -v /containerVar` should mount-bind only /hostTmp into /containeTmp. Received %v", hostConfig.Binds) + } else if _, exists := config.Volumes["/containerTmp"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /containerTmp` is missing from volumes. Received %v", config.Volumes) + } else if _, exists := config.Volumes["/containerVar"]; !exists { + t.Fatalf("Error parsing volume flags, `-v /containerVar` is missing from volumes. Received %v", config.Volumes) + } + + if config, hostConfig := mustParse(t, ""); hostConfig.Binds != nil { + t.Fatalf("Error parsing volume flags, without volume, nothing should be mount-binded. Received %v", hostConfig.Binds) + } else if len(config.Volumes) != 0 { + t.Fatalf("Error parsing volume flags, without volume, no volume should be present. Received %v", config.Volumes) + } + + mustParse(t, "-v /") + + if _, _, err := parse(t, "-v /:/"); err == nil { + t.Fatalf("Error parsing volume flags, `-v /:/` should fail but didn't") + } + if _, _, err := parse(t, "-v"); err == nil { + t.Fatalf("Error parsing volume flags, `-v` should fail but didn't") + } + if _, _, err := parse(t, "-v /tmp:"); err == nil { + t.Fatalf("Error parsing volume flags, `-v /tmp:` should fail but didn't") + } + if _, _, err := parse(t, "-v /tmp:ro"); err == nil { + t.Fatalf("Error parsing volume flags, `-v /tmp:ro` should fail but didn't") + } + if _, _, err := parse(t, "-v /tmp::"); err == nil { + t.Fatalf("Error parsing volume flags, `-v /tmp::` should fail but didn't") + } + if _, _, err := parse(t, "-v :"); err == nil { + t.Fatalf("Error parsing volume flags, `-v :` should fail but didn't") + } + if _, _, err := parse(t, "-v ::"); err == nil { + t.Fatalf("Error parsing volume flags, `-v ::` should fail but didn't") + } + if _, _, err := parse(t, "-v /tmp:/tmp:/tmp:/tmp"); err == nil { + t.Fatalf("Error parsing volume flags, `-v /tmp:/tmp:/tmp:/tmp` should fail but didn't") + } +} From d370a889c37b762a8ddf7e29a8e666fd7cedec3d Mon Sep 17 00:00:00 2001 From: dkumor Date: Tue, 26 Nov 2013 20:00:13 -0600 Subject: [PATCH 11/23] Deleted references to AUFS AUFS is no longer a dependency (both lxc-docker and lxc-docker-git are >=0.7), and the Arch kernel doesn't need to be replaced with AUFS_friendly. --- docs/sources/installation/archlinux.rst | 7 ------- 1 file changed, 7 deletions(-) diff --git a/docs/sources/installation/archlinux.rst b/docs/sources/installation/archlinux.rst index d6dc23925..d34dc6a20 100644 --- a/docs/sources/installation/archlinux.rst +++ b/docs/sources/installation/archlinux.rst @@ -30,7 +30,6 @@ either AUR package. * bridge-utils * go * iproute2 -* linux-aufs_friendly * lxc Installation @@ -41,9 +40,6 @@ The instructions here assume **yaourt** is installed. See for information on building and installing packages from the AUR if you have not done so before. -Keep in mind that if **linux-aufs_friendly** is not already installed that a -new kernel will be compiled and this can take quite a while. - :: yaourt -S lxc-docker-git @@ -52,9 +48,6 @@ new kernel will be compiled and this can take quite a while. Starting Docker --------------- -Prior to starting docker modify your bootloader to use the -**linux-aufs_friendly** kernel and reboot your system. - There is a systemd service unit created for docker. To start the docker service: :: From b3e8ba19085732d9f633e4cbddb660f5f8dc5462 Mon Sep 17 00:00:00 2001 From: dkumor Date: Tue, 26 Nov 2013 22:07:56 -0600 Subject: [PATCH 12/23] Arch docs: Updated dependencies to match AUR The AUR packages lxc-docker and lxc-docker-git have changed their dependencies. --- docs/sources/installation/archlinux.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/installation/archlinux.rst b/docs/sources/installation/archlinux.rst index d34dc6a20..e41529591 100644 --- a/docs/sources/installation/archlinux.rst +++ b/docs/sources/installation/archlinux.rst @@ -26,8 +26,8 @@ Dependencies Docker depends on several packages which are specified as dependencies in either AUR package. -* aufs3 * bridge-utils +* device-mapper * go * iproute2 * lxc From 45b1e8c2362b3626ccc564496e3093386bb89cd9 Mon Sep 17 00:00:00 2001 From: Bruno Bigras Date: Tue, 26 Nov 2013 16:11:08 -0500 Subject: [PATCH 13/23] Update postgresql's version in example It seems ppa:pitti/postgresql will be deprecated and only apt.postgresql.org has 9.3. --- docs/sources/examples/postgresql_service.rst | 30 +++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/docs/sources/examples/postgresql_service.rst b/docs/sources/examples/postgresql_service.rst index 364977548..82ca8b59c 100644 --- a/docs/sources/examples/postgresql_service.rst +++ b/docs/sources/examples/postgresql_service.rst @@ -45,19 +45,21 @@ Install ``python-software-properties``. apt-get -y install python-software-properties apt-get -y install software-properties-common -Add Pitti's PostgreSQL repository. It contains the most recent stable release -of PostgreSQL i.e. ``9.2``. +Add PostgreSQL's repository. It contains the most recent stable release +of PostgreSQL i.e. ``9.3``. .. code-block:: bash - add-apt-repository ppa:pitti/postgresql + apt-get -y install wget + wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - + echo "deb http://apt.postgresql.org/pub/repos/apt/ precise-pgdg main" > /etc/apt/sources.list.d/pgdg.list apt-get update -Finally, install PostgreSQL 9.2 +Finally, install PostgreSQL 9.3 .. code-block:: bash - apt-get -y install postgresql-9.2 postgresql-client-9.2 postgresql-contrib-9.2 + apt-get -y install postgresql-9.3 postgresql-client-9.3 postgresql-contrib-9.3 Now, create a PostgreSQL superuser role that can create databases and other roles. Following Vagrant's convention the role will be named @@ -76,14 +78,14 @@ role. Adjust PostgreSQL configuration so that remote connections to the database are possible. Make sure that inside -``/etc/postgresql/9.2/main/pg_hba.conf`` you have following line (you will need +``/etc/postgresql/9.3/main/pg_hba.conf`` you have following line (you will need to install an editor, e.g. ``apt-get install vim``): .. code-block:: bash host all all 0.0.0.0/0 md5 -Additionaly, inside ``/etc/postgresql/9.2/main/postgresql.conf`` +Additionaly, inside ``/etc/postgresql/9.3/main/postgresql.conf`` uncomment ``listen_addresses`` so it is as follows: .. code-block:: bash @@ -115,9 +117,9 @@ Finally, run PostgreSQL server via ``docker``. CONTAINER=$(sudo docker run -d -p 5432 \ -t /postgresql \ - /bin/su postgres -c '/usr/lib/postgresql/9.2/bin/postgres \ - -D /var/lib/postgresql/9.2/main \ - -c config_file=/etc/postgresql/9.2/main/postgresql.conf') + /bin/su postgres -c '/usr/lib/postgresql/9.3/bin/postgres \ + -D /var/lib/postgresql/9.3/main \ + -c config_file=/etc/postgresql/9.3/main/postgresql.conf') Connect the PostgreSQL server using ``psql`` (You will need postgres installed on the machine. For ubuntu, use something like @@ -132,7 +134,7 @@ As before, create roles or databases if needed. .. code-block:: bash - psql (9.2.4) + psql (9.3.1) Type "help" for help. docker=# CREATE DATABASE foo OWNER=docker; @@ -160,9 +162,9 @@ container starts. .. code-block:: bash sudo docker commit -run='{"Cmd": \ - ["/bin/su", "postgres", "-c", "/usr/lib/postgresql/9.2/bin/postgres -D \ - /var/lib/postgresql/9.2/main -c \ - config_file=/etc/postgresql/9.2/main/postgresql.conf"], "PortSpecs": ["5432"]}' \ + ["/bin/su", "postgres", "-c", "/usr/lib/postgresql/9.3/bin/postgres -D \ + /var/lib/postgresql/9.3/main -c \ + config_file=/etc/postgresql/9.3/main/postgresql.conf"], "PortSpecs": ["5432"]}' \ /postgresql From now on, just type ``docker run /postgresql`` and From 682a188ead44e93ac3d5cc53c708808fa7d2fe4f Mon Sep 17 00:00:00 2001 From: dkumor Date: Wed, 27 Nov 2013 10:25:30 -0600 Subject: [PATCH 14/23] Arch docs: Added lxc-docker-nightly AUR package, modified deps lxc-docker-nightly installs latest build. Removed go from dependencies, as it is not needed in lxc-docker and lxc-docker-nightly. The -git package will flag go as a dependency upon installation. --- docs/sources/installation/archlinux.rst | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/sources/installation/archlinux.rst b/docs/sources/installation/archlinux.rst index e41529591..ae56badf4 100644 --- a/docs/sources/installation/archlinux.rst +++ b/docs/sources/installation/archlinux.rst @@ -12,26 +12,28 @@ Arch Linux .. include:: install_unofficial.inc Installing on Arch Linux is not officially supported but can be handled via -either of the following AUR packages: +one of the following AUR packages: * `lxc-docker `_ * `lxc-docker-git `_ +* `lxc-docker-nightly `_ The lxc-docker package will install the latest tagged version of docker. The lxc-docker-git package will build from the current master branch. +The lxc-docker-nightly package will install the latest build. Dependencies ------------ Docker depends on several packages which are specified as dependencies in -either AUR package. +the AUR packages. The core dependencies are: * bridge-utils * device-mapper -* go * iproute2 * lxc + Installation ------------ @@ -42,7 +44,7 @@ done so before. :: - yaourt -S lxc-docker-git + yaourt -S lxc-docker Starting Docker From 4e826e99b278ef3cc438553e958727397b4d9ee7 Mon Sep 17 00:00:00 2001 From: Paul Nasrat Date: Wed, 27 Nov 2013 12:55:15 -0500 Subject: [PATCH 15/23] Performance of deleteImageAndChildren. Don't walk the file system for parents each time we recurse. Fixes #2852 --- server.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/server.go b/server.go index 4f5e5a2cd..3fcbdbea9 100644 --- a/server.go +++ b/server.go @@ -1406,19 +1406,15 @@ func (srv *Server) ContainerDestroy(name string, removeVolume, removeLink bool) var ErrImageReferenced = errors.New("Image referenced by a repository") -func (srv *Server) deleteImageAndChildren(id string, imgs *[]APIRmi) error { +func (srv *Server) deleteImageAndChildren(id string, imgs *[]APIRmi, byParents map[string][]*Image) error { // If the image is referenced by a repo, do not delete if len(srv.runtime.repositories.ByID()[id]) != 0 { return ErrImageReferenced } // If the image is not referenced but has children, go recursive referenced := false - byParents, err := srv.runtime.graph.ByParent() - if err != nil { - return err - } for _, img := range byParents[id] { - if err := srv.deleteImageAndChildren(img.ID, imgs); err != nil { + if err := srv.deleteImageAndChildren(img.ID, imgs, byParents); err != nil { if err != ErrImageReferenced { return err } @@ -1430,7 +1426,7 @@ func (srv *Server) deleteImageAndChildren(id string, imgs *[]APIRmi) error { } // If the image is not referenced and has no children, remove it - byParents, err = srv.runtime.graph.ByParent() + byParents, err := srv.runtime.graph.ByParent() if err != nil { return err } @@ -1455,8 +1451,12 @@ func (srv *Server) deleteImageParents(img *Image, imgs *[]APIRmi) error { if err != nil { return err } + byParents, err := srv.runtime.graph.ByParent() + if err != nil { + return err + } // Remove all children images - if err := srv.deleteImageAndChildren(img.Parent, imgs); err != nil { + if err := srv.deleteImageAndChildren(img.Parent, imgs, byParents); err != nil { return err } return srv.deleteImageParents(parent, imgs) @@ -1498,7 +1498,7 @@ func (srv *Server) deleteImage(img *Image, repoName, tag string) ([]APIRmi, erro } } if len(srv.runtime.repositories.ByID()[img.ID]) == 0 { - if err := srv.deleteImageAndChildren(img.ID, &imgs); err != nil { + if err := srv.deleteImageAndChildren(img.ID, &imgs, nil); err != nil { if err != ErrImageReferenced { return imgs, err } From 2c27da881807b6c63ef29e8033241a17c76f5ab7 Mon Sep 17 00:00:00 2001 From: Alexis THOMAS Date: Thu, 28 Nov 2013 00:39:06 +0100 Subject: [PATCH 16/23] Restore 'save' paragraph --- docs/sources/commandline/cli.rst | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/sources/commandline/cli.rst b/docs/sources/commandline/cli.rst index 9635675a7..e35475ae9 100644 --- a/docs/sources/commandline/cli.rst +++ b/docs/sources/commandline/cli.rst @@ -797,7 +797,7 @@ Known Issues (kill) -link="": Remove the link instead of the actual container Known Issues (rm) -~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~ * :issue:`197` indicates that ``docker kill`` may leave directories behind and make it difficult to remove the container. @@ -881,8 +881,15 @@ containers will not be deleted. -name="": Assign the specified name to the container. If no name is specific docker will generate a random name -P=false: Publish all exposed ports to the host interfaces -Examples --------- +Known Issues (run -volumes-from) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* :issue:`2702`: "lxc-start: Permission denied - failed to mount" + could indicate a permissions problem with AppArmor. Please see the + issue for a workaround. + +Examples: +~~~~~~~~~ .. code-block:: bash @@ -974,16 +981,10 @@ id may be optionally suffixed with ``:ro`` or ``:rw`` to mount the volumes in read-only or read-write mode, respectively. By default, the volumes are mounted in the same mode (rw or ro) as the reference container. -Known Issues (run -volumes-from) -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -* :issue:`2702`: "lxc-start: Permission denied - failed to mount" - could indicate a permissions problem with AppArmor. Please see the - issue for a workaround. - .. _cli_save: ``save`` +--------- :: From c226ab6d9edb6b74547348c309c633f207bd4aa4 Mon Sep 17 00:00:00 2001 From: Ulysse Carion Date: Wed, 23 Oct 2013 09:19:15 -0700 Subject: [PATCH 17/23] Document setting up Vagrant-docker with the remote API --- contrib/vagrant-docker/README.md | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/contrib/vagrant-docker/README.md b/contrib/vagrant-docker/README.md index fa28e448e..836f50305 100644 --- a/contrib/vagrant-docker/README.md +++ b/contrib/vagrant-docker/README.md @@ -17,3 +17,34 @@ meaning you can use Vagrant to control Docker containers. * [docker-provider](https://github.com/fgrehm/docker-provider) * [vagrant-shell](https://github.com/destructuring/vagrant-shell) + +## Setting up Vagrant-docker with the Remote API + +The initial Docker upstart script will not work because it runs on `127.0.0.1`, which is not accessible to the host machine. Instead, we need to change the script to connect to `0.0.0.0`. To do this, modify `/etc/init/docker.conf` to look like this: + +``` +description "Docker daemon" + +start on filesystem and started lxc-net +stop on runlevel [!2345] + +respawn + +script + /usr/bin/docker -d -H=tcp://0.0.0.0:4243/ +end script +``` + +Once that's done, you need to set up a SSH tunnel between your host machine and the vagrant machine that's running Docker. This can be done by running the following command in a host terminal: + +``` +ssh -L 4243:localhost:4243 -p 2222 vagrant@localhost +``` + +(The first 4243 is what your host can connect to, the second 4243 is what port Docker is running on in the vagrant machine, and the 2222 is the port Vagrant is providing for SSH. If VirtualBox is the VM you're using, you can see what value "2222" should be by going to: Network > Adapter 1 > Advanced > Port Forwarding in the VirtualBox GUI.) + +Note that because the port has been changed, to run docker commands from within the command line you must run them like this: + +``` +sudo docker -H 0.0.0.0:4243 < commands for docker > +``` From f1e44e0b0c6eff011c596008c75f7fdff09dbf5e Mon Sep 17 00:00:00 2001 From: John Warwick Date: Thu, 28 Nov 2013 09:55:15 -0500 Subject: [PATCH 18/23] Remove explanation of removed argument --- docs/sources/examples/hello_world.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/sources/examples/hello_world.rst b/docs/sources/examples/hello_world.rst index 99eaa2c48..c25070fcd 100644 --- a/docs/sources/examples/hello_world.rst +++ b/docs/sources/examples/hello_world.rst @@ -131,8 +131,6 @@ Attach to the container to see the results in real-time. - **"docker attach**" This will allow us to attach to a background process to see what is going on. -- **"-sig-proxy=true"** Proxify all received signal to the process - (even in non-tty mode) - **$CONTAINER_ID** The Id of the container we want to attach too. Exit from the container attachment by pressing Control-C. From d47507791e14908e78cf38d415a9863c9ef75c5e Mon Sep 17 00:00:00 2001 From: Danny Yates Date: Thu, 28 Nov 2013 16:42:29 +0000 Subject: [PATCH 19/23] Stop invalid calls to Registry This code was resulting in a call for /v1/images///ancestry which the Registry doesn't understand. Furthermore, it was masking the original error. --- server.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/server.go b/server.go index 3641e2fdc..b4635c844 100644 --- a/server.go +++ b/server.go @@ -1011,16 +1011,9 @@ func (srv *Server) ImagePull(localName string, tag string, out io.Writer, sf *ut localName = remoteName } - err = srv.pullRepository(r, out, localName, remoteName, tag, endpoint, sf, parallel) - if err == registry.ErrLoginRequired { + if err = srv.pullRepository(r, out, localName, remoteName, tag, endpoint, sf, parallel); err != nil { return err } - if err != nil { - if err := srv.pullImage(r, out, remoteName, endpoint, nil, sf); err != nil { - return err - } - return nil - } return nil } From 533067bba47acec0f65dc7e499944b9402bafb04 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 28 Nov 2013 10:37:03 -0800 Subject: [PATCH 20/23] Rename file for consistency --- graphdriver/devmapper/{attachLoopback.go => attach_loopback.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename graphdriver/devmapper/{attachLoopback.go => attach_loopback.go} (100%) diff --git a/graphdriver/devmapper/attachLoopback.go b/graphdriver/devmapper/attach_loopback.go similarity index 100% rename from graphdriver/devmapper/attachLoopback.go rename to graphdriver/devmapper/attach_loopback.go From 9f46779d42c9b90a70c3c434d03a4502070e1b6d Mon Sep 17 00:00:00 2001 From: Paul Nasrat Date: Tue, 19 Nov 2013 14:25:17 -0500 Subject: [PATCH 21/23] Wire in pprof handlers. Based on http://stackoverflow.com/questions/19591065/profiling-go-web-application-built-with-gorillas-mux-with-net-http-pprof --- api.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/api.go b/api.go index 2880d0e8b..aadd79e3c 100644 --- a/api.go +++ b/api.go @@ -15,6 +15,7 @@ import ( "mime" "net" "net/http" + "net/http/pprof" "os" "os/exec" "regexp" @@ -1037,9 +1038,21 @@ func makeHttpHandler(srv *Server, logging bool, localMethod string, localRoute s } } +func AttachProfiler(router *mux.Router) { + router.HandleFunc("/debug/pprof/", pprof.Index) + router.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + router.HandleFunc("/debug/pprof/profile", pprof.Profile) + router.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + router.HandleFunc("/debug/pprof/heap", pprof.Handler("heap").ServeHTTP) + router.HandleFunc("/debug/pprof/goroutine", pprof.Handler("goroutine").ServeHTTP) + router.HandleFunc("/debug/pprof/threadcreate", pprof.Handler("threadcreate").ServeHTTP) +} + func createRouter(srv *Server, logging bool) (*mux.Router, error) { r := mux.NewRouter() - + if os.Getenv("DEBUG") != "" { + AttachProfiler(r) + } m := map[string]map[string]HttpApiFunc{ "GET": { "/events": getEvents, From a990b3aeb9640d35e73f99030e03e58e71924ed5 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 28 Nov 2013 11:02:53 -0800 Subject: [PATCH 22/23] Correct comments --- graphdriver/devmapper/attach_loopback.go | 4 ++-- graphdriver/devmapper/sys.go | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/graphdriver/devmapper/attach_loopback.go b/graphdriver/devmapper/attach_loopback.go index 0d556d81c..f91bd2016 100644 --- a/graphdriver/devmapper/attach_loopback.go +++ b/graphdriver/devmapper/attach_loopback.go @@ -44,7 +44,7 @@ func openNextAvailableLoopback(index int, sparseFile *osFile) (loopFile *osFile, continue } - // Open the targeted loopback (use OpenFile because Open sets O_CLOEXEC) + // OpenFile adds O_CLOEXEC loopFile, err = osOpenFile(target, osORdWr, 0644) if err != nil { utils.Errorf("Error openning loopback device: %s", err) @@ -89,7 +89,7 @@ func attachLoopDevice(sparseName string) (loop *osFile, err error) { utils.Debugf("Error retrieving the next available loopback: %s", err) } - // Open the given sparse file (use OpenFile because Open sets O_CLOEXEC) + // OpenFile adds O_CLOEXEC sparseFile, err := osOpenFile(sparseName, osORdWr, 0644) if err != nil { utils.Errorf("Error openning sparse file %s: %s", sparseName, err) diff --git a/graphdriver/devmapper/sys.go b/graphdriver/devmapper/sys.go index 9cf124a9a..e06165c82 100644 --- a/graphdriver/devmapper/sys.go +++ b/graphdriver/devmapper/sys.go @@ -34,9 +34,7 @@ var ( osRename = os.Rename osReadlink = os.Readlink - execRun = func(name string, args ...string) error { - return exec.Command(name, args...).Run() - } + execRun = func(name string, args ...string) error { return exec.Command(name, args...).Run() } ) const ( From 261bd0d187cbc8f650d77e5c93872332c290b164 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 28 Nov 2013 11:53:09 -0800 Subject: [PATCH 23/23] Improve devmapper unit tests with syscall/ioctl --- graphdriver/devmapper/devmapper_wrapper.go | 7 +- graphdriver/devmapper/driver_test.go | 124 +++++++++++---------- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/graphdriver/devmapper/devmapper_wrapper.go b/graphdriver/devmapper/devmapper_wrapper.go index 1b636d392..3c195f72e 100644 --- a/graphdriver/devmapper/devmapper_wrapper.go +++ b/graphdriver/devmapper/devmapper_wrapper.go @@ -54,21 +54,24 @@ type ( ) // FIXME: Make sure the values are defined in C +// IOCTL consts const ( + BlkGetSize64 = C.BLKGETSIZE64 + LoopSetFd = C.LOOP_SET_FD LoopCtlGetFree = C.LOOP_CTL_GET_FREE LoopGetStatus64 = C.LOOP_GET_STATUS64 LoopSetStatus64 = C.LOOP_SET_STATUS64 LoopClrFd = C.LOOP_CLR_FD LoopSetCapacity = C.LOOP_SET_CAPACITY +) +const ( LoFlagsAutoClear = C.LO_FLAGS_AUTOCLEAR LoFlagsReadOnly = C.LO_FLAGS_READ_ONLY LoFlagsPartScan = C.LO_FLAGS_PARTSCAN LoKeySize = C.LO_KEY_SIZE LoNameSize = C.LO_NAME_SIZE - - BlkGetSize64 = C.BLKGETSIZE64 ) var ( diff --git a/graphdriver/devmapper/driver_test.go b/graphdriver/devmapper/driver_test.go index 3204575dd..3950152fb 100644 --- a/graphdriver/devmapper/driver_test.go +++ b/graphdriver/devmapper/driver_test.go @@ -55,12 +55,6 @@ func denyAllDevmapper() { DmGetNextTarget = func(task *CDmTask, next uintptr, start, length *uint64, target, params *string) uintptr { panic("DmGetNextTarget: this method should not be called here") } - DmAttachLoopDevice = func(filename string, fd *int) string { - panic("DmAttachLoopDevice: this method should not be called here") - } - DmGetBlockSize = func(fd uintptr) (int64, sysErrno) { - panic("DmGetBlockSize: this method should not be called here") - } DmUdevWait = func(cookie uint) int { panic("DmUdevWait: this method should not be called here") } @@ -76,9 +70,6 @@ func denyAllDevmapper() { DmTaskDestroy = func(task *CDmTask) { panic("DmTaskDestroy: this method should not be called here") } - GetBlockSize = func(fd uintptr, size *uint64) sysErrno { - panic("GetBlockSize: this method should not be called here") - } LogWithErrnoInit = func() { panic("LogWithErrnoInit: this method should not be called here") } @@ -155,11 +146,10 @@ func (r Set) Assert(t *testing.T, names ...string) { func TestInit(t *testing.T) { var ( - calls = make(Set) - devicesAttached = make(Set) - taskMessages = make(Set) - taskTypes = make(Set) - home = mkTestDirectory(t) + calls = make(Set) + taskMessages = make(Set) + taskTypes = make(Set) + home = mkTestDirectory(t) ) defer osRemoveAll(home) @@ -233,29 +223,6 @@ func TestInit(t *testing.T) { taskMessages[message] = true return 1 } - var ( - fakeDataLoop = "/dev/loop42" - fakeMetadataLoop = "/dev/loop43" - fakeDataLoopFd = 42 - fakeMetadataLoopFd = 43 - ) - var attachCount int - DmAttachLoopDevice = func(filename string, fd *int) string { - calls["DmAttachLoopDevice"] = true - if _, exists := devicesAttached[filename]; exists { - t.Fatalf("Already attached %s", filename) - } - devicesAttached[filename] = true - // This will crash if fd is not dereferenceable - if attachCount == 0 { - attachCount++ - *fd = fakeDataLoopFd - return fakeDataLoop - } else { - *fd = fakeMetadataLoopFd - return fakeMetadataLoop - } - } DmTaskDestroy = func(task *CDmTask) { calls["DmTaskDestroy"] = true expectedTask := &task1 @@ -263,14 +230,6 @@ func TestInit(t *testing.T) { t.Fatalf("Wrong libdevmapper call\nExpected: DmTaskDestroy(%v)\nReceived: DmTaskDestroy(%v)\n", expectedTask, task) } } - fakeBlockSize := int64(4242 * 512) - DmGetBlockSize = func(fd uintptr) (int64, sysErrno) { - calls["DmGetBlockSize"] = true - if expectedFd := uintptr(42); fd != expectedFd { - t.Fatalf("Wrong libdevmapper call\nExpected: DmGetBlockSize(%v)\nReceived: DmGetBlockSize(%v)\n", expectedFd, fd) - } - return fakeBlockSize, 0 - } DmTaskAddTarget = func(task *CDmTask, start, size uint64, ttype, params string) int { calls["DmTaskSetTarget"] = true expectedTask := &task1 @@ -345,11 +304,9 @@ func TestInit(t *testing.T) { "DmTaskSetName", "DmTaskRun", "DmTaskGetInfo", - "DmAttachLoopDevice", "DmTaskDestroy", "execRun", "DmTaskCreate", - "DmGetBlockSize", "DmTaskSetTarget", "DmTaskSetCookie", "DmUdevWait", @@ -357,7 +314,6 @@ func TestInit(t *testing.T) { "DmTaskSetMessage", "DmTaskSetAddNode", ) - devicesAttached.Assert(t, path.Join(home, "devicemapper", "data"), path.Join(home, "devicemapper", "metadata")) taskTypes.Assert(t, "0", "6", "17") taskMessages.Assert(t, "create_thin 0", "set_transaction_id 0 1") } @@ -408,17 +364,9 @@ func mockAllDevmapper(calls Set) { calls["DmTaskSetMessage"] = true return 1 } - DmAttachLoopDevice = func(filename string, fd *int) string { - calls["DmAttachLoopDevice"] = true - return "/dev/loop42" - } DmTaskDestroy = func(task *CDmTask) { calls["DmTaskDestroy"] = true } - DmGetBlockSize = func(fd uintptr) (int64, sysErrno) { - calls["DmGetBlockSize"] = true - return int64(4242 * 512), 0 - } DmTaskAddTarget = func(task *CDmTask, start, size uint64, ttype, params string) int { calls["DmTaskSetTarget"] = true return 1 @@ -489,6 +437,32 @@ func TestDriverCreate(t *testing.T) { return false, nil } + sysSyscall = func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err syscall.Errno) { + calls["sysSyscall"] = true + if trap != sysSysIoctl { + t.Fatalf("Unexpected syscall. Expecting SYS_IOCTL, received: %d", trap) + } + switch a2 { + case LoopSetFd: + calls["ioctl.loopsetfd"] = true + case LoopCtlGetFree: + calls["ioctl.loopctlgetfree"] = true + case LoopGetStatus64: + calls["ioctl.loopgetstatus"] = true + case LoopSetStatus64: + calls["ioctl.loopsetstatus"] = true + case LoopClrFd: + calls["ioctl.loopclrfd"] = true + case LoopSetCapacity: + calls["ioctl.loopsetcapacity"] = true + case BlkGetSize64: + calls["ioctl.blkgetsize"] = true + default: + t.Fatalf("Unexpected IOCTL. Received %d", a2) + } + return 0, 0, 0 + } + func() { d := newDriver(t) @@ -498,16 +472,18 @@ func TestDriverCreate(t *testing.T) { "DmTaskSetName", "DmTaskRun", "DmTaskGetInfo", - "DmAttachLoopDevice", "execRun", "DmTaskCreate", - "DmGetBlockSize", "DmTaskSetTarget", "DmTaskSetCookie", "DmUdevWait", "DmTaskSetSector", "DmTaskSetMessage", "DmTaskSetAddNode", + "sysSyscall", + "ioctl.blkgetsize", + "ioctl.loopsetfd", + "ioctl.loopsetstatus", ) if err := d.Create("1", ""); err != nil { @@ -579,6 +555,32 @@ func TestDriverRemove(t *testing.T) { return false, nil } + sysSyscall = func(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err syscall.Errno) { + calls["sysSyscall"] = true + if trap != sysSysIoctl { + t.Fatalf("Unexpected syscall. Expecting SYS_IOCTL, received: %d", trap) + } + switch a2 { + case LoopSetFd: + calls["ioctl.loopsetfd"] = true + case LoopCtlGetFree: + calls["ioctl.loopctlgetfree"] = true + case LoopGetStatus64: + calls["ioctl.loopgetstatus"] = true + case LoopSetStatus64: + calls["ioctl.loopsetstatus"] = true + case LoopClrFd: + calls["ioctl.loopclrfd"] = true + case LoopSetCapacity: + calls["ioctl.loopsetcapacity"] = true + case BlkGetSize64: + calls["ioctl.blkgetsize"] = true + default: + t.Fatalf("Unexpected IOCTL. Received %d", a2) + } + return 0, 0, 0 + } + func() { d := newDriver(t) @@ -588,16 +590,18 @@ func TestDriverRemove(t *testing.T) { "DmTaskSetName", "DmTaskRun", "DmTaskGetInfo", - "DmAttachLoopDevice", "execRun", "DmTaskCreate", - "DmGetBlockSize", "DmTaskSetTarget", "DmTaskSetCookie", "DmUdevWait", "DmTaskSetSector", "DmTaskSetMessage", "DmTaskSetAddNode", + "sysSyscall", + "ioctl.blkgetsize", + "ioctl.loopsetfd", + "ioctl.loopsetstatus", ) if err := d.Create("1", ""); err != nil {