Browse code

Don't sort plugin mounts slice

This was added as part of a53930a04fa81b082aa78e66b342ff19cc63cc5f with
the intent to sort the mounts in the plugin config, but this was sorting
*all* the mounts from the default OCI spec which is problematic.

In reality we don't need to sort this because we are only adding a
self-binded mount to flag it as rshared.

We may want to look at sorting the plugin mounts before they are added
to the OCI spec in the future, but for now I think the existing behavior
is fine since the plugin author has control of the order (except for the
propagated mount).

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2018/03/28 03:03:23
Showing 8 changed files
... ...
@@ -155,6 +155,38 @@ func makePluginBundle(inPath string, opts ...CreateOpt) (io.ReadCloser, error) {
155 155
 	if err := os.MkdirAll(filepath.Join(inPath, "rootfs", filepath.Dir(p.Entrypoint[0])), 0755); err != nil {
156 156
 		return nil, errors.Wrap(err, "error creating plugin rootfs dir")
157 157
 	}
158
+
159
+	// Ensure the mount target paths exist
160
+	for _, m := range p.Mounts {
161
+		var stat os.FileInfo
162
+		if m.Source != nil {
163
+			stat, err = os.Stat(*m.Source)
164
+			if err != nil && !os.IsNotExist(err) {
165
+				return nil, err
166
+			}
167
+		}
168
+
169
+		if stat == nil || stat.IsDir() {
170
+			var mode os.FileMode = 0755
171
+			if stat != nil {
172
+				mode = stat.Mode()
173
+			}
174
+			if err := os.MkdirAll(filepath.Join(inPath, "rootfs", m.Destination), mode); err != nil {
175
+				return nil, errors.Wrap(err, "error preparing plugin mount destination path")
176
+			}
177
+		} else {
178
+			if err := os.MkdirAll(filepath.Join(inPath, "rootfs", filepath.Dir(m.Destination)), 0755); err != nil {
179
+				return nil, errors.Wrap(err, "error preparing plugin mount destination dir")
180
+			}
181
+			f, err := os.Create(filepath.Join(inPath, "rootfs", m.Destination))
182
+			if err != nil && !os.IsExist(err) {
183
+				return nil, errors.Wrap(err, "error preparing plugin mount destination file")
184
+			}
185
+			if f != nil {
186
+				f.Close()
187
+			}
188
+		}
189
+	}
158 190
 	if err := archive.NewDefaultArchiver().CopyFileWithTar(cfg.binPath, filepath.Join(inPath, "rootfs", p.Entrypoint[0])); err != nil {
159 191
 		return nil, errors.Wrap(err, "error copying plugin binary to rootfs path")
160 192
 	}
161 193
new file mode 100644
... ...
@@ -0,0 +1 @@
0
+package cmd
0 1
new file mode 100644
... ...
@@ -0,0 +1,19 @@
0
+package main
1
+
2
+import (
3
+	"net"
4
+	"net/http"
5
+)
6
+
7
+func main() {
8
+	l, err := net.Listen("unix", "/run/docker/plugins/plugin.sock")
9
+	if err != nil {
10
+		panic(err)
11
+	}
12
+
13
+	server := http.Server{
14
+		Addr:    l.Addr().String(),
15
+		Handler: http.NewServeMux(),
16
+	}
17
+	server.Serve(l)
18
+}
0 19
new file mode 100644
... ...
@@ -0,0 +1 @@
0
+package main
0 1
new file mode 100644
... ...
@@ -0,0 +1,73 @@
0
+package volumes
1
+
2
+import (
3
+	"context"
4
+	"os"
5
+	"os/exec"
6
+	"path/filepath"
7
+	"testing"
8
+	"time"
9
+
10
+	"github.com/docker/docker/api/types"
11
+	"github.com/docker/docker/integration-cli/fixtures/plugin"
12
+	"github.com/docker/docker/pkg/locker"
13
+	"github.com/pkg/errors"
14
+)
15
+
16
+var pluginBuildLock = locker.New()
17
+
18
+// ensurePlugin makes the that a plugin binary has been installed on the system.
19
+// Plugins that have not been installed are built from `cmd/<name>`.
20
+func ensurePlugin(t *testing.T, name string) string {
21
+	pluginBuildLock.Lock(name)
22
+	defer pluginBuildLock.Unlock(name)
23
+
24
+	goPath := os.Getenv("GOPATH")
25
+	if goPath == "" {
26
+		goPath = "/go"
27
+	}
28
+	installPath := filepath.Join(goPath, "bin", name)
29
+	if _, err := os.Stat(installPath); err == nil {
30
+		return installPath
31
+	}
32
+
33
+	goBin, err := exec.LookPath("go")
34
+	if err != nil {
35
+		t.Fatal(err)
36
+	}
37
+
38
+	cmd := exec.Command(goBin, "build", "-o", installPath, "./"+filepath.Join("cmd", name))
39
+	cmd.Env = append(cmd.Env, "CGO_ENABLED=0")
40
+	if out, err := cmd.CombinedOutput(); err != nil {
41
+		t.Fatal(errors.Wrapf(err, "error building basic plugin bin: %s", string(out)))
42
+	}
43
+
44
+	return installPath
45
+}
46
+
47
+func withSockPath(name string) func(*plugin.Config) {
48
+	return func(cfg *plugin.Config) {
49
+		cfg.Interface.Socket = name
50
+	}
51
+}
52
+
53
+func createPlugin(t *testing.T, client plugin.CreateClient, alias, bin string, opts ...plugin.CreateOpt) {
54
+	pluginBin := ensurePlugin(t, bin)
55
+
56
+	opts = append(opts, withSockPath("plugin.sock"))
57
+	opts = append(opts, plugin.WithBinary(pluginBin))
58
+
59
+	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
60
+	err := plugin.Create(ctx, client, alias, opts...)
61
+	cancel()
62
+
63
+	if err != nil {
64
+		t.Fatal(err)
65
+	}
66
+}
67
+
68
+func asVolumeDriver(cfg *plugin.Config) {
69
+	cfg.Interface.Types = []types.PluginInterfaceType{
70
+		{Capability: "volumedriver", Prefix: "docker", Version: "1.0"},
71
+	}
72
+}
0 73
new file mode 100644
... ...
@@ -0,0 +1,34 @@
0
+package volumes // import "github.com/docker/docker/integration/plugin/volumes"
1
+
2
+import (
3
+	"fmt"
4
+	"os"
5
+	"testing"
6
+
7
+	"github.com/docker/docker/internal/test/environment"
8
+)
9
+
10
+var (
11
+	testEnv *environment.Execution
12
+)
13
+
14
+const dockerdBinary = "dockerd"
15
+
16
+func TestMain(m *testing.M) {
17
+	var err error
18
+	testEnv, err = environment.New()
19
+	if err != nil {
20
+		fmt.Println(err)
21
+		os.Exit(1)
22
+	}
23
+	if testEnv.OSType != "linux" {
24
+		os.Exit(0)
25
+	}
26
+	err = environment.EnsureFrozenImagesLinux(testEnv)
27
+	if err != nil {
28
+		fmt.Println(err)
29
+		os.Exit(1)
30
+	}
31
+	testEnv.Print()
32
+	os.Exit(m.Run())
33
+}
0 34
new file mode 100644
... ...
@@ -0,0 +1,56 @@
0
+package volumes
1
+
2
+import (
3
+	"context"
4
+	"io/ioutil"
5
+	"os"
6
+	"testing"
7
+
8
+	"github.com/docker/docker/api/types"
9
+	"github.com/docker/docker/integration-cli/daemon"
10
+	"github.com/docker/docker/integration-cli/fixtures/plugin"
11
+	"github.com/gotestyourself/gotestyourself/assert"
12
+)
13
+
14
+// TestPluginWithDevMounts tests very specific regression caused by mounts ordering
15
+// (sorted in the daemon). See #36698
16
+func TestPluginWithDevMounts(t *testing.T) {
17
+	t.Parallel()
18
+
19
+	d := daemon.New(t, "", dockerdBinary, daemon.Config{})
20
+	d.Start(t, "--iptables=false")
21
+	defer d.Stop(t)
22
+
23
+	client, err := d.NewClient()
24
+	assert.Assert(t, err)
25
+	ctx := context.Background()
26
+
27
+	testDir, err := ioutil.TempDir("", "test-dir")
28
+	assert.Assert(t, err)
29
+	defer os.RemoveAll(testDir)
30
+
31
+	createPlugin(t, client, "test", "dummy", asVolumeDriver, func(c *plugin.Config) {
32
+		root := "/"
33
+		dev := "/dev"
34
+		mounts := []types.PluginMount{
35
+			{Type: "bind", Source: &root, Destination: "/host", Options: []string{"rbind"}},
36
+			{Type: "bind", Source: &dev, Destination: "/dev", Options: []string{"rbind"}},
37
+			{Type: "bind", Source: &testDir, Destination: "/etc/foo", Options: []string{"rbind"}},
38
+		}
39
+		c.PluginConfig.Mounts = append(c.PluginConfig.Mounts, mounts...)
40
+		c.PropagatedMount = "/propagated"
41
+		c.Network = types.PluginConfigNetwork{Type: "host"}
42
+		c.IpcHost = true
43
+	})
44
+
45
+	err = client.PluginEnable(ctx, "test", types.PluginEnableOptions{Timeout: 30})
46
+	assert.Assert(t, err)
47
+	defer func() {
48
+		err := client.PluginRemove(ctx, "test", types.PluginRemoveOptions{Force: true})
49
+		assert.Check(t, err)
50
+	}()
51
+
52
+	p, _, err := client.PluginInspectWithRaw(ctx, "test")
53
+	assert.Assert(t, err)
54
+	assert.Assert(t, p.Enabled)
55
+}
... ...
@@ -4,7 +4,6 @@ import (
4 4
 	"os"
5 5
 	"path/filepath"
6 6
 	"runtime"
7
-	"sort"
8 7
 	"strings"
9 8
 
10 9
 	"github.com/docker/docker/api/types"
... ...
@@ -138,9 +137,5 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) {
138 138
 		p.modifyRuntimeSpec(&s)
139 139
 	}
140 140
 
141
-	sort.Slice(s.Mounts, func(i, j int) bool {
142
-		return s.Mounts[i].Destination < s.Mounts[j].Destination
143
-	})
144
-
145 141
 	return &s, nil
146 142
 }