make container's shared memory configurable via annotation #1052
Conversation
a26e6e5
to
f258981
Compare
|
Just out of curiosity: Why do we do this in gcs than doing it during container document creation? |
|
@ambarve I think I'd said that we handle a similar annotation that changes the oci spec (io.microsoft.virtualmachine.lcow.privileged) in the guest so it might make sense to do the same for this. Maybe we should move both out to the initial document creation if it makes sense |
|
Why not filter out the initial mount in CRI the way /dev and /sys/fs/cgroup mounts are filtered out today? |
all the way to CRI? or? |
I started off by doing something like this: anmaxvl/cri@cb87112, which needs to be changed since we'll eventually have 2 configs: 1 for sandbox /dev/shm (which needs to be implemented first) and the second one is for container (current PR). |
same question as to @dcantah. |
No just in the LCOW container document setup in the shim I was thinking. I feel like checking/having logic for our annotations in our cri fork is gonna be a pain if we ever move off of it (but I'm guilty of doing the same). |
looking a bit more into it, we could update the |
|
@anmaxvl I don't mind it here, don't know about others. |
|
I don't really have an issue with it being done in the gcs but maybe we should have a function that sets up all of these annotation specific overrides instead of them being inline, as there's now three. e.g. func <validateAnnotationsOrSomething>(spec *oci.Spec) error {
if val, ok := spec.Annotations["io.microsoft.container.storage.shm.size-kb"]; ok {
sz, err := strconv.ParseInt(val, 10, 64)
if err != nil {
return errors.Wrap(err, "/dev/shm size must be a valid integer")
}
if sz <= 0 {
return errors.Errorf("/dev/shm size must be a positive integer, got: %d", sz)
}
size := fmt.Sprintf("size=%dk", sz)
mt := oci.Mount{
Destination: "/dev/shm",
Type: "tmpfs",
Source: "shm",
Options: []string{"nosuid", "noexec", "nodev", "mode=1777", size},
}
spec.Mounts = removeMount("/dev/shm", spec.Mounts)
spec.Mounts = append(spec.Mounts, mt)
}
// Check if we need to do any capability/device mappings
if spec.Annotations["io.microsoft.virtualmachine.lcow.privileged"] == "true" {
log.G(ctx).Debug("'io.microsoft.virtualmachine.lcow.privileged' set for privileged container")
// Add all host devices
hostDevices, err := devices.HostDevices()
if err != nil {
return err
}
for _, hostDevice := range hostDevices {
addLinuxDeviceToSpec(ctx, hostDevice, spec, false)
}
// Set the cgroup access
spec.Linux.Resources.Devices = []oci.LinuxDeviceCgroup{
{
Allow: true,
Access: "rwm",
},
}
} else {
tempLinuxDevices := spec.Linux.Devices
spec.Linux.Devices = []oci.LinuxDevice{}
for _, ld := range tempLinuxDevices {
hostDevice, err := devices.DeviceFromPath(ld.Path, "rwm")
if err != nil {
return err
}
addLinuxDeviceToSpec(ctx, hostDevice, spec, true)
}
}
if userstr, ok := spec.Annotations["io.microsoft.lcow.userstr"]; ok {
if err := setUserStr(spec, userstr); err != nil {
return err
}
}
return nil
} |
f258981
to
652fc5b
Compare
652fc5b
to
3aa927b
Compare
3aa927b
to
019a2da
Compare
add annotation "io.microsoft.container.storage.shm.size-kb" to set container's /dev/shm tmpfs size. this overrides any existing /dev/shm mounts in the spec additionally move the annotations parsing logic into a separate function Signed-off-by: Maksim An <[email protected]>
019a2da
to
461cf9f
Compare
Related work items: microsoft#930, microsoft#962, microsoft#1004, microsoft#1008, microsoft#1039, microsoft#1045, microsoft#1046, microsoft#1047, microsoft#1052, microsoft#1053, microsoft#1054, microsoft#1057, microsoft#1058, microsoft#1060, microsoft#1061, microsoft#1063, microsoft#1064, microsoft#1068, microsoft#1069, microsoft#1070, microsoft#1071, microsoft#1074, microsoft#1078, microsoft#1079, microsoft#1081, microsoft#1082, microsoft#1083, microsoft#1084, microsoft#1088, microsoft#1090, microsoft#1091, microsoft#1093, microsoft#1094, microsoft#1096, microsoft#1098, microsoft#1099, microsoft#1102, microsoft#1103, microsoft#1105, microsoft#1106, microsoft#1108, microsoft#1109, microsoft#1115, microsoft#1116, microsoft#1122, microsoft#1123, microsoft#1126
This PR adds annotation
"io.microsoft.container.storage.shm.size-kb"toset container's
/dev/shmtmpfs size, which overrides any existing/dev/shmmount in the spec
Signed-off-by: Maksim An [email protected]