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?

Why not filter out the initial mount in CRI the way /dev and /sys/fs/cgroup mounts are filtered out today?

@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

all the way to CRI? or?

Why not filter out the initial mount in CRI the way /dev and /sys/fs/cgroup mounts are filtered out today?

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).

Just out of curiosity: Why do we do this in gcs than doing it during container document creation?

same question as to @dcantah.

all the way to CRI? or?

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).

all the way to CRI? or?

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 /dev/shm mount as part of the document creation, however we cannot move some of the other customizations as easily (e.g. privileged container part), since they check the devices on the UVM in order to share them within the container. that being said, do we really want to move the /dev/shm logic out?

@anmaxvl I don't mind it here, don't know about others.

@katiewasnothere, @ambarve ?

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
}