Merge pull request containerd#29 from klihub/fixes/sort-adjusted-mounts

pkg/api/mount.go

@@ -47,7 +47,6 @@ func (m *Mount) ToOCI(propagationQuery *string) rspec.Mount {
 		Destination: m.Destination,
 		Type:        m.Type,
 		Source:      m.Source,
-		Options:     []string{},
 	}
 	for _, opt := range m.Options {
 		o.Options = append(o.Options, opt)

pkg/runtime-tools/generate/generate.go

@@ -18,6 +18,9 @@ package generate
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
+	"sort"
 	"strings"
 
 	rspec "github.com/opencontainers/runtime-spec/specs-go"
@@ -319,19 +322,20 @@ func (g *Generator) AdjustDevices(devices []*nri.LinuxDevice) {
 
 // AdjustMounts adjusts the mounts in the OCI Spec.
 func (g *Generator) AdjustMounts(mounts []*nri.Mount) error {
-	var (
-		propagation string
-	)
+	if len(mounts) == 0 {
+		return nil
+	}
 
+	propagation := ""
 	for _, m := range mounts {
 		if destination, marked := m.IsMarkedForRemoval(); marked {
 			g.RemoveMount(destination)
 			continue
 		}
+
 		g.RemoveMount(m.Destination)
 
 		mnt := m.ToOCI(&propagation)
-
 		switch propagation {
 		case "rprivate":
 		case "rshared":
@@ -354,10 +358,60 @@ func (g *Generator) AdjustMounts(mounts []*nri.Mount) error {
 		}
 		g.AddMount(mnt)
 	}
+	g.sortMounts()
 
 	return nil
 }
 
+// sortMounts sorts the mounts in the generated OCI Spec.
+func (g *Generator) sortMounts() {
+	mounts := g.Generator.Mounts()
+	g.Generator.ClearMounts()
+	sort.Sort(orderedMounts(mounts))
+
+	// TODO(klihub): This is now a bit ugly maybe we should introduce a
+	// SetMounts([]rspec.Mount) to runtime-tools/generate.Generator. That
+	// could also take care of properly sorting the mount slice.
+
+	g.Generator.Config.Mounts = mounts
+}
+
+// orderedMounts defines how to sort an OCI Spec Mount slice.
+// This is the almost the same implementation sa used by CRI-O and Docker,
+// with a minor tweak for stable sorting order (easier to test):
+//
+//	https://github.com/moby/moby/blob/17.05.x/daemon/volumes.go#L26
+type orderedMounts []rspec.Mount
+
+// Len returns the number of mounts. Used in sorting.
+func (m orderedMounts) Len() int {
+	return len(m)
+}
+
+// Less returns true if the number of parts (a/b/c would be 3 parts) in the
+// mount indexed by parameter 1 is less than that of the mount indexed by
+// parameter 2. Used in sorting.
+func (m orderedMounts) Less(i, j int) bool {
+	ip, jp := m.parts(i), m.parts(j)
+	if ip < jp {
+		return true
+	}
+	if jp < ip {
+		return false
+	}
+	return m[i].Destination < m[j].Destination
+}
+
+// Swap swaps two items in an array of mounts. Used in sorting
+func (m orderedMounts) Swap(i, j int) {
+	m[i], m[j] = m[j], m[i]
+}
+
+// parts returns the number of parts in the destination of a mount. Used in sorting.
+func (m orderedMounts) parts(i int) int {
+	return strings.Count(filepath.Clean(m[i].Destination), string(os.PathSeparator))
+}
+
 func nopFilter(m map[string]string) (map[string]string, error) {
 	return m, nil
 }

pkg/runtime-tools/generate/generate_suite_test.go

@@ -212,6 +212,60 @@ var _ = Describe("Adjustment", func() {
 			Expect(spec).To(Equal(makeSpec(withCPUSetMems("5,6"))))
 		})
 	})
+
+	When("has mounts", func() {
+		It("it sorts the Spec mount slice", func() {
+			var (
+				spec   = makeSpec()
+				adjust = &api.ContainerAdjustment{
+					Mounts: []*api.Mount{
+						{
+							Destination: "/a/b/c/d/e",
+							Source:      "/host/e",
+						},
+						{
+							Destination: "/a/b/c",
+							Source:      "/host/c",
+						},
+						{
+							Destination: "/a/b",
+							Source:      "/host/b",
+						},
+						{
+							Destination: "/a",
+							Source:      "/host/a",
+						},
+					},
+				}
+			)
+
+			rg := &rgen.Generator{Config: spec}
+			xg := xgen.SpecGenerator(rg)
+
+			Expect(xg).ToNot(BeNil())
+			Expect(xg.Adjust(adjust)).To(Succeed())
+			Expect(spec).To(Equal(makeSpec(
+				withMounts([]rspec.Mount{
+					{
+						Destination: "/a",
+						Source:      "/host/a",
+					},
+					{
+						Destination: "/a/b",
+						Source:      "/host/b",
+					},
+					{
+						Destination: "/a/b/c",
+						Source:      "/host/c",
+					},
+					{
+						Destination: "/a/b/c/d/e",
+						Source:      "/host/e",
+					},
+				}),
+			)))
+		})
+	})
 })
 
 type specOption func(*rspec.Spec)
@@ -321,6 +375,12 @@ func withCPUSetMems(v string) specOption {
 	}
 }
 
+func withMounts(mounts []rspec.Mount) specOption {
+	return func(spec *rspec.Spec) {
+		spec.Mounts = append(spec.Mounts, mounts...)
+	}
+}
+
 func makeSpec(options ...specOption) *rspec.Spec {
 	spec := &rspec.Spec{
 		Process: &rspec.Process{},