Skip to content

Commit ec90839

Browse files
cpuguy83cpuguy83
committed
Don't sort plugin mounts slice
This was added as part of a53930a 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 <[email protected]>
1 parent ed7b642 commit ec90839

8 files changed

Lines changed: 216 additions & 5 deletions

File tree

‎integration-cli/fixtures/plugin/plugin.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,38 @@ func makePluginBundle(inPath string, opts ...CreateOpt) (io.ReadCloser, error) {
155155
if err := os.MkdirAll(filepath.Join(inPath, "rootfs", filepath.Dir(p.Entrypoint[0])), 0755); err != nil {
156156
return nil, errors.Wrap(err, "error creating plugin rootfs dir")
157157
}
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+
}
158190
if err := archive.NewDefaultArchiver().CopyFileWithTar(cfg.binPath, filepath.Join(inPath, "rootfs", p.Entrypoint[0])); err != nil {
159191
return nil, errors.Wrap(err, "error copying plugin binary to rootfs path")
160192
}

‎integration/plugin/volumes/cmd/cmd_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package cmd

‎integration/plugin/volumes/cmd/dummy/main.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package main
2+
3+
import (
4+
"net"
5+
"net/http"
6+
)
7+
8+
func main() {
9+
l, err := net.Listen("unix", "/run/docker/plugins/plugin.sock")
10+
if err != nil {
11+
panic(err)
12+
}
13+
14+
server := http.Server{
15+
Addr: l.Addr().String(),
16+
Handler: http.NewServeMux(),
17+
}
18+
server.Serve(l)
19+
}

‎integration/plugin/volumes/cmd/dummy/main_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
package main

‎integration/plugin/volumes/helpers_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package volumes
2+
3+
import (
4+
"context"
5+
"os"
6+
"os/exec"
7+
"path/filepath"
8+
"testing"
9+
"time"
10+
11+
"github.com/docker/docker/api/types"
12+
"github.com/docker/docker/integration-cli/fixtures/plugin"
13+
"github.com/docker/docker/pkg/locker"
14+
"github.com/pkg/errors"
15+
)
16+
17+
var pluginBuildLock = locker.New()
18+
19+
// ensurePlugin makes the that a plugin binary has been installed on the system.
20+
// Plugins that have not been installed are built from `cmd/<name>`.
21+
func ensurePlugin(t *testing.T, name string) string {
22+
pluginBuildLock.Lock(name)
23+
defer pluginBuildLock.Unlock(name)
24+
25+
goPath := os.Getenv("GOPATH")
26+
if goPath == "" {
27+
goPath = "/go"
28+
}
29+
installPath := filepath.Join(goPath, "bin", name)
30+
if _, err := os.Stat(installPath); err == nil {
31+
return installPath
32+
}
33+
34+
goBin, err := exec.LookPath("go")
35+
if err != nil {
36+
t.Fatal(err)
37+
}
38+
39+
cmd := exec.Command(goBin, "build", "-o", installPath, "./"+filepath.Join("cmd", name))
40+
cmd.Env = append(cmd.Env, "CGO_ENABLED=0")
41+
if out, err := cmd.CombinedOutput(); err != nil {
42+
t.Fatal(errors.Wrapf(err, "error building basic plugin bin: %s", string(out)))
43+
}
44+
45+
return installPath
46+
}
47+
48+
func withSockPath(name string) func(*plugin.Config) {
49+
return func(cfg *plugin.Config) {
50+
cfg.Interface.Socket = name
51+
}
52+
}
53+
54+
func createPlugin(t *testing.T, client plugin.CreateClient, alias, bin string, opts ...plugin.CreateOpt) {
55+
pluginBin := ensurePlugin(t, bin)
56+
57+
opts = append(opts, withSockPath("plugin.sock"))
58+
opts = append(opts, plugin.WithBinary(pluginBin))
59+
60+
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
61+
err := plugin.Create(ctx, client, alias, opts...)
62+
cancel()
63+
64+
if err != nil {
65+
t.Fatal(err)
66+
}
67+
}
68+
69+
func asVolumeDriver(cfg *plugin.Config) {
70+
cfg.Interface.Types = []types.PluginInterfaceType{
71+
{Capability: "volumedriver", Prefix: "docker", Version: "1.0"},
72+
}
73+
}

‎integration/plugin/volumes/main_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package volumes // import "github.com/docker/docker/integration/plugin/volumes"
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"testing"
7+
8+
"github.com/docker/docker/internal/test/environment"
9+
)
10+
11+
var (
12+
testEnv *environment.Execution
13+
)
14+
15+
const dockerdBinary = "dockerd"
16+
17+
func TestMain(m *testing.M) {
18+
var err error
19+
testEnv, err = environment.New()
20+
if err != nil {
21+
fmt.Println(err)
22+
os.Exit(1)
23+
}
24+
if testEnv.OSType != "linux" {
25+
os.Exit(0)
26+
}
27+
err = environment.EnsureFrozenImagesLinux(testEnv)
28+
if err != nil {
29+
fmt.Println(err)
30+
os.Exit(1)
31+
}
32+
testEnv.Print()
33+
os.Exit(m.Run())
34+
}

‎integration/plugin/volumes/mounts_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package volumes
2+
3+
import (
4+
"context"
5+
"io/ioutil"
6+
"os"
7+
"testing"
8+
9+
"github.com/docker/docker/api/types"
10+
"github.com/docker/docker/integration-cli/daemon"
11+
"github.com/docker/docker/integration-cli/fixtures/plugin"
12+
"github.com/gotestyourself/gotestyourself/assert"
13+
)
14+
15+
// TestPluginWithDevMounts tests very specific regression caused by mounts ordering
16+
// (sorted in the daemon). See #36698
17+
func TestPluginWithDevMounts(t *testing.T) {
18+
t.Parallel()
19+
20+
d := daemon.New(t, "", dockerdBinary, daemon.Config{})
21+
d.Start(t, "--iptables=false")
22+
defer d.Stop(t)
23+
24+
client, err := d.NewClient()
25+
assert.Assert(t, err)
26+
ctx := context.Background()
27+
28+
testDir, err := ioutil.TempDir("", "test-dir")
29+
assert.Assert(t, err)
30+
defer os.RemoveAll(testDir)
31+
32+
createPlugin(t, client, "test", "dummy", asVolumeDriver, func(c *plugin.Config) {
33+
root := "/"
34+
dev := "/dev"
35+
mounts := []types.PluginMount{
36+
{Type: "bind", Source: &root, Destination: "/host", Options: []string{"rbind"}},
37+
{Type: "bind", Source: &dev, Destination: "/dev", Options: []string{"rbind"}},
38+
{Type: "bind", Source: &testDir, Destination: "/etc/foo", Options: []string{"rbind"}},
39+
}
40+
c.PluginConfig.Mounts = append(c.PluginConfig.Mounts, mounts...)
41+
c.PropagatedMount = "/propagated"
42+
c.Network = types.PluginConfigNetwork{Type: "host"}
43+
c.IpcHost = true
44+
})
45+
46+
err = client.PluginEnable(ctx, "test", types.PluginEnableOptions{Timeout: 30})
47+
assert.Assert(t, err)
48+
defer func() {
49+
err := client.PluginRemove(ctx, "test", types.PluginRemoveOptions{Force: true})
50+
assert.Check(t, err)
51+
}()
52+
53+
p, _, err := client.PluginInspectWithRaw(ctx, "test")
54+
assert.Assert(t, err)
55+
assert.Assert(t, p.Enabled)
56+
}

‎plugin/v2/plugin_linux.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"os"
55
"path/filepath"
66
"runtime"
7-
"sort"
87
"strings"
98

109
"github.com/docker/docker/api/types"
@@ -138,9 +137,5 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) {
138137
p.modifyRuntimeSpec(&s)
139138
}
140139

141-
sort.Slice(s.Mounts, func(i, j int) bool {
142-
return s.Mounts[i].Destination < s.Mounts[j].Destination
143-
})
144-
145140
return &s, nil
146141
}

0 commit comments

Comments
 (0)