1 files changed, 34 insertions, 30 deletions

diff --git a/pkg/shim/utils/volumes.go b/pkg/shim/utils/volumes.go
index 52a428179..6bc75139d 100644
--- a/pkg/shim/utils/volumes.go
+++ b/pkg/shim/utils/volumes.go
@@ -15,9 +15,7 @@
 package utils
 
 import (
-	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"path/filepath"
 	"strings"
 
@@ -89,18 +87,16 @@ func isVolumePath(volume, path string) (bool, error) {
 }
 
 // UpdateVolumeAnnotations add necessary OCI annotations for gvisor
-// volume optimization.
-func UpdateVolumeAnnotations(bundle string, s *specs.Spec) error {
-	var (
-		uid string
-		err error
-	)
+// volume optimization. Returns true if the spec was modified.
+func UpdateVolumeAnnotations(s *specs.Spec) (bool, error) {
+	var uid string
 	if IsSandbox(s) {
+		var err error
 		uid, err = podUID(s)
 		if err != nil {
 			// Skip if we can't get pod UID, because this doesn't work
 			// for containerd 1.1.
-			return nil
+			return false, nil
 		}
 	}
 	var updated bool
@@ -116,40 +112,48 @@ func UpdateVolumeAnnotations(bundle string, s *specs.Spec) error {
 			// This is a sandbox.
 			path, err := volumePath(volume, uid)
 			if err != nil {
-				return fmt.Errorf("get volume path for %q: %w", volume, err)
+				return false, fmt.Errorf("get volume path for %q: %w", volume, err)
 			}
 			s.Annotations[volumeSourceKey(volume)] = path
 			updated = true
 		} else {
 			// This is a container.
 			for i := range s.Mounts {
-				// An error is returned for sandbox if source
-				// annotation is not successfully applied, so
-				// it is guaranteed that the source annotation
-				// for sandbox has already been successfully
-				// applied at this point.
+				// An error is returned for sandbox if source annotation is not
+				// successfully applied, so it is guaranteed that the source annotation
+				// for sandbox has already been successfully applied at this point.
 				//
-				// The volume name is unique inside a pod, so
-				// matching without podUID is fine here.
+				// The volume name is unique inside a pod, so matching without podUID
+				// is fine here.
 				//
-				// TODO: Pass podUID down to shim for containers to do
-				// more accurate matching.
+				// TODO: Pass podUID down to shim for containers to do more accurate
+				// matching.
 				if yes, _ := isVolumePath(volume, s.Mounts[i].Source); yes {
-					// gVisor requires the container mount type to match
-					// sandbox mount type.
-					s.Mounts[i].Type = v
+					// Container mount type must match the sandbox's mount type.
+					changeMountType(&s.Mounts[i], v)
 					updated = true
 				}
 			}
 		}
 	}
-	if !updated {
-		return nil
-	}
-	// Update bundle.
-	b, err := json.Marshal(s)
-	if err != nil {
-		return err
+	return updated, nil
+}
+
+func changeMountType(m *specs.Mount, newType string) {
+	m.Type = newType
+
+	// OCI spec allows bind mounts to be specified in options only. So if new type
+	// is not bind, remove bind/rbind from options.
+	//
+	// "For bind mounts (when options include either bind or rbind), the type is
+	// a dummy, often "none" (not listed in /proc/filesystems)."
+	if newType != "bind" {
+		newOpts := make([]string, 0, len(m.Options))
+		for _, opt := range m.Options {
+			if opt != "rbind" && opt != "bind" {
+				newOpts = append(newOpts, opt)
+			}
+		}
+		m.Options = newOpts
 	}
-	return ioutil.WriteFile(filepath.Join(bundle, "config.json"), b, 0666)
 }