summaryrefslogtreecommitdiffhomepage
path: root/pkg/shim/utils
diff options
context:
space:
mode:
Diffstat (limited to 'pkg/shim/utils')
-rw-r--r--pkg/shim/utils/annotations.go6
-rw-r--r--pkg/shim/utils/utils.go20
-rw-r--r--pkg/shim/utils/volumes.go64
-rw-r--r--pkg/shim/utils/volumes_test.go190
4 files changed, 157 insertions, 123 deletions
diff --git a/pkg/shim/utils/annotations.go b/pkg/shim/utils/annotations.go
index 1e9d3f365..c744800bb 100644
--- a/pkg/shim/utils/annotations.go
+++ b/pkg/shim/utils/annotations.go
@@ -19,7 +19,9 @@ package utils
// These are vendor due to import conflicts.
const (
sandboxLogDirAnnotation = "io.kubernetes.cri.sandbox-log-directory"
- containerTypeAnnotation = "io.kubernetes.cri.container-type"
+ // ContainerTypeAnnotation is they key that defines sandbox or container.
+ ContainerTypeAnnotation = "io.kubernetes.cri.container-type"
containerTypeSandbox = "sandbox"
- containerTypeContainer = "container"
+ // ContainerTypeContainer is the value for container.
+ ContainerTypeContainer = "container"
)
diff --git a/pkg/shim/utils/utils.go b/pkg/shim/utils/utils.go
index 7b1cd983e..f183b1bbc 100644
--- a/pkg/shim/utils/utils.go
+++ b/pkg/shim/utils/utils.go
@@ -18,19 +18,16 @@ package utils
import (
"encoding/json"
"io/ioutil"
- "os"
"path/filepath"
specs "github.com/opencontainers/runtime-spec/specs-go"
)
+const configFilename = "config.json"
+
// ReadSpec reads OCI spec from the bundle directory.
func ReadSpec(bundle string) (*specs.Spec, error) {
- f, err := os.Open(filepath.Join(bundle, "config.json"))
- if err != nil {
- return nil, err
- }
- b, err := ioutil.ReadAll(f)
+ b, err := ioutil.ReadFile(filepath.Join(bundle, configFilename))
if err != nil {
return nil, err
}
@@ -41,9 +38,18 @@ func ReadSpec(bundle string) (*specs.Spec, error) {
return &spec, nil
}
+// WriteSpec writes OCI spec to the bundle directory.
+func WriteSpec(bundle string, spec *specs.Spec) error {
+ b, err := json.Marshal(spec)
+ if err != nil {
+ return err
+ }
+ return ioutil.WriteFile(filepath.Join(bundle, configFilename), b, 0666)
+}
+
// IsSandbox checks whether a container is a sandbox container.
func IsSandbox(spec *specs.Spec) bool {
- t, ok := spec.Annotations[containerTypeAnnotation]
+ t, ok := spec.Annotations[ContainerTypeAnnotation]
return !ok || t == containerTypeSandbox
}
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)
}
diff --git a/pkg/shim/utils/volumes_test.go b/pkg/shim/utils/volumes_test.go
index 3e02c6151..5db43cdf1 100644
--- a/pkg/shim/utils/volumes_test.go
+++ b/pkg/shim/utils/volumes_test.go
@@ -15,11 +15,9 @@
package utils
import (
- "encoding/json"
"fmt"
"io/ioutil"
"os"
- "path/filepath"
"reflect"
"testing"
@@ -47,60 +45,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 +115,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 +137,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 +157,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,63 +173,63 @@ 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,
- containerTypeAnnotation: containerTypeSandbox,
+ ContainerTypeAnnotation: containerTypeSandbox,
},
},
expected: &specs.Spec{
Annotations: map[string]string{
sandboxLogDirAnnotation: testLogDirPath,
- containerTypeAnnotation: containerTypeSandbox,
+ ContainerTypeAnnotation: containerTypeSandbox,
},
},
},
{
- desc: "no volume annotations for container",
+ name: "no volume annotations for container",
spec: &specs.Spec{
Mounts: []specs.Mount{
{
@@ -248,7 +246,7 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeContainer,
+ ContainerTypeAnnotation: ContainerTypeContainer,
},
},
expected: &specs.Spec{
@@ -267,17 +265,51 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
},
Annotations: map[string]string{
- containerTypeAnnotation: containerTypeContainer,
+ ContainerTypeAnnotation: ContainerTypeContainer,
},
},
},
+ {
+ 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) {
- bundle, err := ioutil.TempDir(dir, "test-bundle")
- if err != nil {
- t.Fatalf("Create test bundle: %v", err)
- }
- err = UpdateVolumeAnnotations(bundle, test.spec)
+ t.Run(test.name, func(t *testing.T) {
+ updated, err := UpdateVolumeAnnotations(test.spec)
if test.expectErr {
if err == nil {
t.Fatal("Expected error, but got nil")
@@ -290,18 +322,8 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
if !reflect.DeepEqual(test.expected, test.spec) {
t.Fatalf("Expected %+v, got %+v", test.expected, test.spec)
}
- if test.expectUpdate {
- b, err := ioutil.ReadFile(filepath.Join(bundle, "config.json"))
- if err != nil {
- t.Fatalf("Read spec from bundle: %v", err)
- }
- var spec specs.Spec
- if err := json.Unmarshal(b, &spec); err != nil {
- t.Fatalf("Unmarshal spec: %v", err)
- }
- if !reflect.DeepEqual(test.expected, &spec) {
- t.Fatalf("Expected %+v, got %+v", test.expected, &spec)
- }
+ if test.expectUpdate != updated {
+ t.Errorf("Expected %v, got %v", test.expected, updated)
}
})
}
generated by cgit v1.2.3 (git 2.39.1) at 2025-04-11 16:24:07 +0000