netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Delete lifecycle mount annotation.

Browse Source 
Earlier, NewPodMountHints() was modifying the mount hints based on the
lifecycle. This change moves that annotation modification work to runsc shim.
It is more consistent for the shim to do all OCI spec modification work. Also
added more documentation about how EmptyDir is optimized in runsc.

This also allows us to delete the lifecycle mount annotation as it is no longer
used anywhere else.

PiperOrigin-RevId: 574600949
This commit is contained in:
Ayush Ranjan
2023-10-18 14:36:27 -07:00
committed by gVisor bot
parent e77deec462
commit f286d71166
6 changed files with 74 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/shim/utils/volumes.go
+27 -18
View File
@@ -81,11 +81,6 @@ func volumeSourceKey(volume string) string {
return volumeKeyPrefix + volume + ".source"
}
// volumeLifecycleKey constructs the annotation key for volume lifecycle.
func volumeLifecycleKey(volume string) string {
return volumeKeyPrefix + volume + ".lifecycle"
}
// volumePath searches the volume path in the kubelet pod directory.
func volumePath(volume, uid string) (string, error) {
// TODO: Support subpath when gvisor supports pod volume bind mount.
@@ -109,6 +104,23 @@ func isVolumePath(volume, path string) (bool, error) {
// UpdateVolumeAnnotations add necessary OCI annotations for gvisor
// volume optimization. Returns true if the spec was modified.
//
// Note about EmptyDir handling:
// The admission controller sets mount annotations for EmptyDir as follows:
// - For EmptyDir volumes with medium=Memory, the "type" field is set to tmpfs.
// - For EmptyDir volumes with medium="", the "type" field is set to bind.
//
// The container spec has EmptyDir mount points as bind mounts. This method
// modifies the spec as follows:
// - The "type" mount annotation for all EmptyDirs is changed to tmpfs.
// - The mount type in spec.Mounts[i].Type is changed as follows:
// - For EmptyDir volumes with medium=Memory, we change it to tmpfs.
// - For EmptyDir volumes with medium="", we leave it as a bind mount.
// - (Essentially we set it to what the admission controller said.)
//
// runsc should use these two setting to infer EmptyDir medium:
// - tmpfs annotation type + tmpfs mount type = memory-backed EmptyDir
// - tmpfs annotation type + bind mount type = disk-backed EmptyDir
func UpdateVolumeAnnotations(s *specs.Spec) (bool, error) {
var uid string
if IsSandbox(s) {
@@ -130,25 +142,22 @@ func UpdateVolumeAnnotations(s *specs.Spec) (bool, error) {
}
volume := volumeName(k)
if uid != "" {
// This is a sandbox. Add source and lifecycle annotations for volumes.
// This is the root (first) container. Mount annotations are only
// consumed from this container's spec. So fix mount annotations by:
// 1. Adding source annotation.
// 2. Fixing type annotation.
path, err := volumePath(volume, uid)
if err != nil {
return false, fmt.Errorf("get volume path for %q: %w", volume, err)
}
s.Annotations[volumeSourceKey(volume)] = path
// TODO(b/142076984): Remove the lifecycle setting logic after it has
// been adopted in GKE admission plugin.
lifecycleKey := volumeLifecycleKey(volume)
if _, ok := s.Annotations[lifecycleKey]; !ok {
// Only set lifecycle annotation if not already set.
if strings.Contains(path, emptyDirVolumesDir) {
// Emptydir is created and destroyed with the pod.
s.Annotations[lifecycleKey] = "pod"
}
if strings.Contains(path, emptyDirVolumesDir) {
s.Annotations[k] = "tmpfs" // See note about EmptyDir.
}
updated = true
} else {
// This is a container.
// This is a sub-container. Mount annotations are ignored. So no need to
// bother fixing those.
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
@@ -160,7 +169,8 @@ func UpdateVolumeAnnotations(s *specs.Spec) (bool, error) {
// TODO: Pass podUID down to shim for containers to do more accurate
// matching.
if yes, _ := isVolumePath(volume, s.Mounts[i].Source); yes {
// Container mount type must match the sandbox's mount type.
// Container mount type must match the mount type specified by
// admission controller. See note about EmptyDir.
specutils.ChangeMountType(&s.Mounts[i], v)
updated = true
}
@@ -211,7 +221,6 @@ func configureShm(s *specs.Spec) (bool, error) {
s.Annotations[volumeKeyPrefix+devshmName+".source"] = m.Source
s.Annotations[volumeKeyPrefix+devshmName+".type"] = devshmType
s.Annotations[volumeKeyPrefix+devshmName+".share"] = "pod"
s.Annotations[volumeKeyPrefix+devshmName+".lifecycle"] = "pod"
// Given that we don't have visibility into mount options for all
// containers, assume broad access for the master mount (it's tmpfs
// inside the sandbox anyways) and apply options to subcontainers as
pkg/shim/utils/volumes_test.go
+28 -33
View File
@@ -64,13 +64,12 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
expected: &specs.Spec{
Annotations: map[string]string{
sandboxLogDirAnnotation: testLogDirPath,
ContainerTypeAnnotation: containerTypeSandbox,
volumeKeyPrefix + testVolumeName + ".share": "pod",
volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
volumeKeyPrefix + testVolumeName + ".options": "ro",
volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
volumeKeyPrefix + testVolumeName + ".lifecycle": "pod",
sandboxLogDirAnnotation: testLogDirPath,
ContainerTypeAnnotation: containerTypeSandbox,
volumeKeyPrefix + testVolumeName + ".share": "pod",
volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
volumeKeyPrefix + testVolumeName + ".options": "ro",
volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
},
},
expectUpdate: true,
@@ -88,13 +87,12 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
expected: &specs.Spec{
Annotations: map[string]string{
sandboxLogDirAnnotation: testLegacyLogDirPath,
ContainerTypeAnnotation: containerTypeSandbox,
volumeKeyPrefix + testVolumeName + ".share": "pod",
volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
volumeKeyPrefix + testVolumeName + ".options": "ro",
volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
volumeKeyPrefix + testVolumeName + ".lifecycle": "pod",
sandboxLogDirAnnotation: testLegacyLogDirPath,
ContainerTypeAnnotation: containerTypeSandbox,
volumeKeyPrefix + testVolumeName + ".share": "pod",
volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
volumeKeyPrefix + testVolumeName + ".options": "ro",
volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
},
},
expectUpdate: true,
@@ -275,12 +273,11 @@ 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,
volumeKeyPrefix + testVolumeName + ".lifecycle": "pod",
ContainerTypeAnnotation: ContainerTypeContainer,
volumeKeyPrefix + testVolumeName + ".share": "pod",
volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
volumeKeyPrefix + testVolumeName + ".options": "ro",
volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
},
Mounts: []specs.Mount{
{
@@ -293,12 +290,11 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
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,
volumeKeyPrefix + testVolumeName + ".lifecycle": "pod",
ContainerTypeAnnotation: ContainerTypeContainer,
volumeKeyPrefix + testVolumeName + ".share": "pod",
volumeKeyPrefix + testVolumeName + ".type": "tmpfs",
volumeKeyPrefix + testVolumeName + ".options": "ro",
volumeKeyPrefix + testVolumeName + ".source": testVolumePath,
},
Mounts: []specs.Mount{
{
@@ -329,13 +325,12 @@ func TestUpdateVolumeAnnotations(t *testing.T) {
},
expected: &specs.Spec{
Annotations: map[string]string{
sandboxLogDirAnnotation: testLogDirPath,
ContainerTypeAnnotation: containerTypeSandbox,
volumeKeyPrefix + devshmName + ".share": "pod",
volumeKeyPrefix + devshmName + ".type": "tmpfs",
volumeKeyPrefix + devshmName + ".options": "rw",
volumeKeyPrefix + devshmName + ".source": testVolumePath,
volumeKeyPrefix + devshmName + ".lifecycle": "pod",
sandboxLogDirAnnotation: testLogDirPath,
ContainerTypeAnnotation: containerTypeSandbox,
volumeKeyPrefix + devshmName + ".share": "pod",
volumeKeyPrefix + devshmName + ".type": "tmpfs",
volumeKeyPrefix + devshmName + ".options": "rw",
volumeKeyPrefix + devshmName + ".source": testVolumePath,
},
Mounts: []specs.Mount{
{
runsc/boot/mount_hints.go
+3 -65
View File
@@ -62,37 +62,6 @@ func (s ShareType) String() string {
}
}
// LifecycleType indicates whether creation/deletion of the volume is tied to
// the pod or container's lifecycle.
type LifecycleType int
const (
// sharedLife indicates that the volume's lifecycle is not tied to the pod.
// The volume persists beyond the pod's life. This is the safe default.
sharedLife LifecycleType = iota
// podLife indicates that the volume's lifecycle is tied to the pod's
// lifecycle. The volume is destroyed with the pod.
podLife
// containerLife indicates that the volume's lifecycle is tied to the
// container's lifecycle. The volume is destroyed with the container.
containerLife
)
func (o LifecycleType) String() string {
switch o {
case sharedLife:
return "shared"
case podLife:
return "pod"
case containerLife:
return "container"
default:
return fmt.Sprintf("invalid lifecycle value %d", o)
}
}
// PodMountHints contains a collection of mountHints for the pod.
type PodMountHints struct {
Mounts map[string]*MountHint `json:"mounts"`
@@ -141,20 +110,6 @@ func NewPodMountHints(spec *specs.Spec) (*PodMountHints, error) {
}
}
// Convert mount types.
for _, m := range mnts {
if m.Mount.Type == Bind &&
// This mount is only accessed within the sandbox.
(m.Share == container || m.Share == pod) &&
// The mount is created and deleted within the pod's lifecycle.
(m.Lifecycle == containerLife || m.Lifecycle == podLife) {
// Use a file-backed tmpfs mount for such a mount, because it is isolated
// to the sandbox and is empty on startup.
log.Infof("Converting %s hint to tmpfs", m.Name)
m.Mount.Type = tmpfs.Name
}
}
return &PodMountHints{Mounts: mnts}, nil
}
@@ -163,10 +118,9 @@ func NewPodMountHints(spec *specs.Spec) (*PodMountHints, error) {
// so that mounts can be correctly shared inside the pod.
// It is part of the sandbox.Sandbox struct, so it must be serializable.
type MountHint struct {
Name string `json:"name"`
Share ShareType `json:"share"`
Mount specs.Mount `json:"mount"`
Lifecycle LifecycleType `json:"lifecycle"`
Name string `json:"name"`
Share ShareType `json:"share"`
Mount specs.Mount `json:"mount"`
}
func (m *MountHint) setField(key, val string) error {
@@ -182,8 +136,6 @@ func (m *MountHint) setField(key, val string) error {
return m.setShare(val)
case "options":
m.Mount.Options = specutils.FilterMountOptions(strings.Split(val, ","))
case "lifecycle":
return m.setLifecycle(val)
default:
return fmt.Errorf("invalid mount annotation: %s=%s", key, val)
}
@@ -214,20 +166,6 @@ func (m *MountHint) setShare(val string) error {
return nil
}
func (m *MountHint) setLifecycle(val string) error {
switch val {
case containerLife.String():
m.Lifecycle = containerLife
case podLife.String():
m.Lifecycle = podLife
case sharedLife.String():
m.Lifecycle = sharedLife
default:
return fmt.Errorf("invalid lifecycle %q", val)
}
return nil
}
// ShouldShareMount returns true if this mount should be configured as a shared
// mount that is shared among multiple containers in a pod.
func (m *MountHint) ShouldShareMount() bool {
runsc/boot/mount_hints_test.go
+5 -12
View File
@@ -29,11 +29,10 @@ func TestPodMountHintsHappy(t *testing.T) {
MountPrefix + "mount1.type": "tmpfs",
MountPrefix + "mount1.share": "pod",
MountPrefix + "mount2.source": "bar",
MountPrefix + "mount2.type": "bind",
MountPrefix + "mount2.share": "container",
MountPrefix + "mount2.options": "rw,private",
MountPrefix + "mount2.lifecycle": "pod",
MountPrefix + "mount2.source": "bar",
MountPrefix + "mount2.type": "bind",
MountPrefix + "mount2.share": "container",
MountPrefix + "mount2.options": "rw,private",
},
}
podHints, err := NewPodMountHints(spec)
@@ -58,9 +57,6 @@ func TestPodMountHintsHappy(t *testing.T) {
if want := []string(nil); !reflect.DeepEqual(want, mount1.Mount.Options) {
t.Errorf("mount1 type, want: %q, got: %q", want, mount1.Mount.Options)
}
if want := sharedLife; want != mount1.Lifecycle {
t.Errorf("mount1 lifecycle, want: %q, got: %q", want, mount1.Lifecycle)
}
mount2 := podHints.Mounts["mount2"]
if want := "mount2"; want != mount2.Name {
@@ -69,7 +65,7 @@ func TestPodMountHintsHappy(t *testing.T) {
if want := "bar"; want != mount2.Mount.Source {
t.Errorf("mount2 source, want: %q, got: %q", want, mount2.Mount.Source)
}
if want := "tmpfs"; want != mount2.Mount.Type {
if want := "bind"; want != mount2.Mount.Type {
t.Errorf("mount2 type, want: %q, got: %q", want, mount2.Mount.Type)
}
if want := container; want != mount2.Share {
@@ -78,9 +74,6 @@ func TestPodMountHintsHappy(t *testing.T) {
if want := []string{"rw", "private"}; !reflect.DeepEqual(want, mount2.Mount.Options) {
t.Errorf("mount2 type, want: %q, got: %q", want, mount2.Mount.Options)
}
if want := podLife; want != mount2.Lifecycle {
t.Errorf("mount2 lifecycle, want: %q, got: %q", want, mount2.Lifecycle)
}
}
func TestPodMountHintsErrors(t *testing.T) {
runsc/boot/vfs_test.go
+10 -11
View File
@@ -47,17 +47,7 @@ func TestGetMountAccessType(t *testing.T) {
want: config.FileAccessShared,
},
{
name: "pod+podLife=shared",
annotations: map[string]string{
MountPrefix + "mount1.source": source,
MountPrefix + "mount1.type": "bind",
MountPrefix + "mount1.share": "pod",
MountPrefix + "mount1.lifecycle": "pod",
},
want: config.FileAccessExclusive,
},
{
name: "shared=shared",
name: "share=shared",
annotations: map[string]string{
MountPrefix + "mount1.source": source,
MountPrefix + "mount1.type": "bind",
@@ -74,6 +64,15 @@ func TestGetMountAccessType(t *testing.T) {
},
want: config.FileAccessShared,
},
{
name: "tmpfs+container=exclusive",
annotations: map[string]string{
MountPrefix + "mount1.source": source,
MountPrefix + "mount1.type": "tmpfs",
MountPrefix + "mount1.share": "container",
},
want: config.FileAccessExclusive,
},
{
name: "tmpfs+pod=exclusive",
annotations: map[string]string{
runsc/container/multi_container_test.go
+1 -2
View File
@@ -131,9 +131,8 @@ func createSharedMount(mount specs.Mount, name string, pod ...*specs.Spec) {
}
for _, spec := range pod {
spec.Annotations[boot.MountPrefix+name+".source"] = mount.Source
spec.Annotations[boot.MountPrefix+name+".type"] = mount.Type
spec.Annotations[boot.MountPrefix+name+".type"] = "tmpfs"
spec.Annotations[boot.MountPrefix+name+".share"] = share
spec.Annotations[boot.MountPrefix+name+".lifecycle"] = "pod"
if len(mount.Options) > 0 {
spec.Annotations[boot.MountPrefix+name+".options"] = strings.Join(mount.Options, ",")
}