Memory swap fix for non supporting kernels

[rajasec]

@LK4D4 @crosbymichael @mrunalp
While I was modifying the swap value in config.json under memory section, it failed to write the value as the current kernel ( 3.19 : Ubuntu 15.04) and failed to start the container. Post that I have to update the grub to include cgroup_enable=memory swapaccount=1 which had "memory.memsw" values. So for the kernel which is not having "memory.memsw" by default, I have added the condition to check whether memory.memsw.limit_in_bytes is available before writing to the cgroup file.

Signed-off-by: Rajasekaran rajasec79@gmail.com

[rajasec]

@LK4D4 @crosbymichael @mrunalp
Some failures during build
W: Failed to fetch http://httpredir.debian.org/debian/dists/jessie/main/binary-amd64/Packages 503 Service Unavailable [IP: 128.31.0.66 80]

[runcom]

maybe move this after if cgroup.MemorySwap > 0 { check

[hqhq]

This is a wider problem than just memory swap, we used to depend on Docker to check if these configs are valid (related kernel configs are enabled or such files are existed), now runC are used without Docker, we probably need something like https://github.com/docker/docker/blob/master/daemon/daemon.go#L1083 in runC.

[rajasec]

@runcom
Fixed the comment.

@hqhq
I would leave to Maintainers to take the call here wrt Platform specific settings which you have mentioned in Docker. It make sense to verify the kernel configs between the platform.

[runcom]

Something like pkg/sysinfo in docker https://github.com/docker/docker/blob/master/pkg/sysinfo/sysinfo_linux.go is needed to properly check if/which cgroups are enabled before writing to subsystem's files

[rajasec]

@runcom
As per sysinfo_linux function cgroupEnabled function is generic function which will handle. In fact this function is doing os.Stat internally for existing of cgroup subsystem file
In this PR, I've handled it explicitly via os.Stat for memory.memsw. This will gracefully ignore the writing to cgroup subsystem file, if the file is not present.
If we are bringing the generic function, every writeFile in each subsystem need to check for cgroup file existence before writing to the file. This will prevent the issues apart from memory swap

[runcom]

@rajasec yes I see, I'm just wondering if cgroups subsystem existence check should be made in libcontainer as you're doing here (I also think apply_systemd should be modified as well in this case) or it should be handled by runc as docker is doing

[rajasec]

@runcom
Agreed. I'll try to get the changes for checking cgroup subsystem file existence, so that each subsystem can check before writing. Let me put through this PR. In that case it wont be superfluous
With this change, additional system call ( os.Stat) is added/called for every cgroup subsystem file writing while container is started

[rajasec]

@runcom @hqhq @LK4D4 @crosbymichael @mrunalp
I have added the generic function to handle both memory swap and cpu. This will gets called before writing to the cgroup file. I have not handled for every cgroup files ( for each subsystem) With this PR, I have added only for memsw and cpuquota, cpu period. I've made the changes in systemd too by calling this util function before applying to cgroup files.

[hqhq]

What about,
logrus.Warn("Your kernel does not support swap memory limit. Limitation discarded.")
Same as others.

[rajasec]

@hqhq
Done.

[runcom]

I'm not fond of logrus calls in libcontainer :/ what about checking and adjusting Config in start.go (maybe) in runc before launching the container? Just thinking

[rajasec]

@runcom
Even I was thinking in validator, where I could validate the configs before starting the container, but since the cgroup paths and mounts happening later, so I thought this place would be appropriate before writing to cgroup file. I've used the logrus to provide the warning and gracefully start the container

[rajasec]

@crosbymichael @runcom @LK4D4 @mrunalp
As per @runcom comments, can we check this config set up during start up on container

First option:
As per this PR, checking the cgroup file in memory.go ( where you will get the cgroup processes complete path)
/sys/fs/cgroup/devices/user.slice/user-1000.slice/session-c2.scope/runc
This complete path is created in if err := os.MkdirAll in memory.go ( Apply function) before setting the cgroup file
This PR addresses via option 1.

Second option
We can check in /sys/fs/cgroup/memory for existence of memory.memsw.limit_in_bytes and correct the config.
During cgroup container creation itself this will correct the config
I've checked this option too, this is also working.

Your thoughts, whether existence of cgroup file in /sys/fs/cgroup/memory suffice enough to correct the config file as per option 2

[crosbymichael]

Closing this to keep the discussion going in #262