Apply memory cgroups to the container. #44
Conversation
|
Does this only restrict memory? |
Yes, CPU is a soft limit in |
| @@ -105,6 +105,9 @@ func (d *Driver) createContainer(image containerd.Image, containerName, containe | |||
| // Set environment variables. | |||
| opts = append(opts, oci.WithEnv(env)) | |||
|
|
|||
| // Set cgroups memory limit. | |||
| opts = append(opts, oci.WithMemoryLimit(uint64(memoryLimit))) | |||
There was a problem hiding this comment.
Isn't this a little dangerous? What if someone puts in -9223372036854775808?
There was a problem hiding this comment.
This value (memoryLimit) is coming from nomad resource stanza.
Does nomad have any resource validations? as in can someone pass a junk value to nomad job and it will accept?
Let me check.
There was a problem hiding this comment.
Looks like nomad does validations on the input:
Case 1: memory = -9223372036854775808
root@ash1-lapp301:~/go/src/github.com/roblox/nomad-driver-containerd/example# nomad job run hog.nomad
Error submitting job: Unexpected response code: 500 (1 error occurred:
* Task group hog-group validation failed: 1 error occurred:
* Task hog-task validation failed: 1 error occurred:
* 1 error occurred:
* 1 error occurred:
* minimum MemoryMB value is 10; got -9223372036854775808
Case 2: memory = 9223372036854775808
root@ash1-lapp301:~/go/src/github.com/roblox/nomad-driver-containerd/example# nomad job run hog.nomad
Error getting job struct: Error parsing job file from hog.nomad: error parsing 'job': strconv.ParseInt: parsing "9223372036854775808": value out of range
There was a problem hiding this comment.
Case 2 is not an ideal error, but at least it allows you to search for the value.
There was a problem hiding this comment.
I guess my next concern would be -1. That representation should be 9223372036854775809 (or something like that). I suppose I can RTFnomadC to find out, but it just seems odd to mix and convert types this way without validation on the value.
|
CPU can be a hard limit with the current Docker driver, and even with a soft limit, it's still applied as a part of the cgroup limits in docker via quota/period. I will approve this and open a new issue. 👍 |
|
To follow up on this, the bounds checking from nomad is here: https://github.com/hashicorp/nomad/blob/master/nomad/structs/structs.go#L2217. I might try to implement some sort of bounds-check the maximum next week to throw a slightly nicer error, akin to the ones thrown for minimum violations. |
Fixes #8