summaryrefslogtreecommitdiffhomepage
path: root/pkg/shim/utils
diff options
context:
space:
mode:
Diffstat (limited to 'pkg/shim/utils')
-rw-r--r--pkg/shim/utils/volumes.go46
-rw-r--r--pkg/shim/utils/volumes_test.go160
2 files changed, 129 insertions, 77 deletions
diff --git a/pkg/shim/utils/volumes.go b/pkg/shim/utils/volumes.go
index 52a428179..cdcb88229 100644
--- a/pkg/shim/utils/volumes.go
+++ b/pkg/shim/utils/volumes.go
@@ -91,11 +91,9 @@ 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
- )
+ 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
@@ -123,21 +121,18 @@ func UpdateVolumeAnnotations(bundle string, s *specs.Spec) error {
} 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
}
}
@@ -153,3 +148,22 @@ func UpdateVolumeAnnotations(bundle string, s *specs.Spec) error {
}
return ioutil.WriteFile(filepath.Join(bundle, "config.json"), b, 0666)
}
+
+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
+ }
+}
diff --git a/pkg/shim/utils/volumes_test.go b/pkg/shim/utils/volumes_test.go
index 3e02c6151..b25c53c73 100644
--- a/pkg/shim/utils/volumes_test.go
+++ b/pkg/shim/utils/volumes_test.go
@@ -47,60 +47,60 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
}
for _, test := range []struct {
- desc string
+ name string
spec *specs.Spec
expected *specs.Spec
expectErr bool
expectUpdate bool
}{
{
- desc: "volume annotations for sandbox",
+ name: "volume annotations for sandbox",
spec: &specs.Spec{
Annotations: map[string]string{
- sandboxLogDirAnnotation: testLogDirPath,
- containerTypeAnnotation: containerTypeSandbox,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ sandboxLogDirAnnotation: testLogDirPath,
+ containerTypeAnnotation: containerTypeSandbox,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
expected: &specs.Spec{
Annotations: map[string]string{
- sandboxLogDirAnnotation: testLogDirPath,
- containerTypeAnnotation: containerTypeSandbox,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
- "dev.gvisor.spec.mount." + testVolumeName + ".source": testVolumePath,
+ sandboxLogDirAnnotation: testLogDirPath,
+ containerTypeAnnotation: containerTypeSandbox,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
+ volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
},
},
expectUpdate: true,
},
{
- desc: "volume annotations for sandbox with legacy log path",
+ name: "volume annotations for sandbox with legacy log path",
spec: &specs.Spec{
Annotations: map[string]string{
- sandboxLogDirAnnotation: testLegacyLogDirPath,
- containerTypeAnnotation: containerTypeSandbox,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ sandboxLogDirAnnotation: testLegacyLogDirPath,
+ containerTypeAnnotation: containerTypeSandbox,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
expected: &specs.Spec{
Annotations: map[string]string{
- sandboxLogDirAnnotation: testLegacyLogDirPath,
- containerTypeAnnotation: containerTypeSandbox,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
- "dev.gvisor.spec.mount." + testVolumeName + ".source": testVolumePath,
+ sandboxLogDirAnnotation: testLegacyLogDirPath,
+ containerTypeAnnotation: containerTypeSandbox,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
+ volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
},
},
expectUpdate: true,
},
{
- desc: "tmpfs: volume annotations for container",
+ name: "tmpfs: volume annotations for container",
spec: &specs.Spec{
Mounts: []specs.Mount{
{
@@ -117,10 +117,10 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeContainer,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ containerTypeAnnotation: containerTypeContainer,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
expected: &specs.Spec{
@@ -139,16 +139,16 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeContainer,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ containerTypeAnnotation: containerTypeContainer,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
expectUpdate: true,
},
{
- desc: "bind: volume annotations for container",
+ name: "bind: volume annotations for container",
spec: &specs.Spec{
Mounts: []specs.Mount{
{
@@ -159,10 +159,10 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeContainer,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "container",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "bind",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ containerTypeAnnotation: containerTypeContainer,
+ volumeKeyPrefix + testVolumeName + ".share": "container",
+ volumeKeyPrefix + testVolumeName + ".type": "bind",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
expected: &specs.Spec{
@@ -175,48 +175,48 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeContainer,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "container",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "bind",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ containerTypeAnnotation: containerTypeContainer,
+ volumeKeyPrefix + testVolumeName + ".share": "container",
+ volumeKeyPrefix + testVolumeName + ".type": "bind",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
expectUpdate: true,
},
{
- desc: "should not return error without pod log directory",
+ name: "should not return error without pod log directory",
spec: &specs.Spec{
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeSandbox,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ containerTypeAnnotation: containerTypeSandbox,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
expected: &specs.Spec{
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeSandbox,
- "dev.gvisor.spec.mount." + testVolumeName + ".share": "pod",
- "dev.gvisor.spec.mount." + testVolumeName + ".type": "tmpfs",
- "dev.gvisor.spec.mount." + testVolumeName + ".options": "ro",
+ containerTypeAnnotation: containerTypeSandbox,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
},
},
},
{
- desc: "should return error if volume path does not exist",
+ name: "should return error if volume path does not exist",
spec: &specs.Spec{
Annotations: map[string]string{
- sandboxLogDirAnnotation: testLogDirPath,
- containerTypeAnnotation: containerTypeSandbox,
- "dev.gvisor.spec.mount.notexist.share": "pod",
- "dev.gvisor.spec.mount.notexist.type": "tmpfs",
- "dev.gvisor.spec.mount.notexist.options": "ro",
+ sandboxLogDirAnnotation: testLogDirPath,
+ containerTypeAnnotation: containerTypeSandbox,
+ volumeKeyPrefix + "notexist.share": "pod",
+ volumeKeyPrefix + "notexist.type": "tmpfs",
+ volumeKeyPrefix + "notexist.options": "ro",
},
},
expectErr: true,
},
{
- desc: "no volume annotations for sandbox",
+ name: "no volume annotations for sandbox",
spec: &specs.Spec{
Annotations: map[string]string{
sandboxLogDirAnnotation: testLogDirPath,
@@ -231,7 +231,7 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
{
- desc: "no volume annotations for container",
+ name: "no volume annotations for container",
spec: &specs.Spec{
Mounts: []specs.Mount{
{
@@ -271,8 +271,46 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
},
+ {
+ name: "bind options removed",
+ spec: &specs.Spec{
+ Annotations: map[string]string{
+ containerTypeAnnotation: containerTypeContainer,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
+ volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
+ },
+ Mounts: []specs.Mount{
+ {
+ Destination: "/dst",
+ Type: "bind",
+ Source: testVolumePath,
+ Options: []string{"ro", "bind", "rbind"},
+ },
+ },
+ },
+ expected: &specs.Spec{
+ Annotations: map[string]string{
+ containerTypeAnnotation: containerTypeContainer,
+ volumeKeyPrefix + testVolumeName + ".share": "pod",
+ volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
+ volumeKeyPrefix + testVolumeName + ".options": "ro",
+ volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
+ },
+ Mounts: []specs.Mount{
+ {
+ Destination: "/dst",
+ Type: "tmpfs",
+ Source: testVolumePath,
+ Options: []string{"ro"},
+ },
+ },
+ },
+ expectUpdate: true,
+ },
} {
- t.Run(test.desc, func(t *testing.T) {
+ t.Run(test.name, func(t *testing.T) {
bundle, err := ioutil.TempDir(dir, "test-bundle")
if err != nil {
t.Fatalf("Create test bundle: %v", err)