[base/kvm] Add KVM enabled base container #949
Conversation
|
This PR should solve #939. |
|
Some slowness is observed during the execution of a simple mininet command such as |
There was a problem hiding this comment.
This is very a first pass. I'd like to see those issues addressed first because I'm wondering if there is a possibility to make this fully modular and if it would be relevant.
Indeed, the added lines seem very tied to the additional container architecture and make code harder to understand. I would know if it's possible to put these additional piece of codes in a kind of hook folder provided in the container image.
| @@ -150,13 +165,14 @@ def create_container(self, image, network_grading, mem_limit, task_path, sockets | |||
| course_common_student_path: {'bind': '/course/common/student', 'mode': 'ro'} | |||
| }, | |||
| runtime=runtime, | |||
| ulimits=[nofile_limit] | |||
| ulimits=[nofile_limit], | |||
| devices=['/dev/kvm'] if kvm and image_requires_kvm else [] | |||
There was a problem hiding this comment.
I fear that unwanted behaviours can happen silently here. If the container image requires kvm and the agent does not support/enable it, no notification will be given. There seems to be no way for the user to know if the agent supports kvm, and, moreover, nothing prevents the job to be assigned to an inappropriate agent.
I'm however more likely to deploy an agent that do support kvm into a cluster of other agents that don't.
For me it would probably be more relevant to implement this as the ssh or nvidia filters : announce another agent type that will allow container images tagged with org.inginious.kvm to be listed as available environment, so that we can ensure appropriate routing and ensure the user selected the right environment.
Implementation of nvidia is here f842bee
However, I realize that implementing a kvm filter, as it may be compatible with other agent type, would require to duplicate all those one to have a kvm-enabled flavour.
Maybe it would be useful to rethink the way this is done and add some kind of feature flags in addition to the agent type, so that we can handle docker or nvidia with ssh or kvm as supported features or not.
There was a problem hiding this comment.
After discussion with @anthonygego, this will be implemented as existing filters (ssh, nvidia, ..).
The features flags common to all the Agents will be grouped in a new data-class.
Front-end side, the grading environment should not include the feature flags in the dropdown and those will be added as check boxes.
| @@ -21,6 +21,7 @@ logger = setup_logger() | |||
| # Check the runtimes | |||
| runtime = sys.argv[1] | |||
| parent_runtime = sys.argv[2] | |||
| kvm_enabled = sys.argv[3] == "True" | |||
There was a problem hiding this comment.
I'd document the args at both sides, here and at the call side in _docker_interface.py (to avoid implementing a useless argumentparser that would be self documenting)
There was a problem hiding this comment.
The arguments managment will be replaced by an argparser.
This PR adds a new base container able to launch a specific Linux kernel within a KVM.
New grading environments should inherit from this container as it is the case for the base container.
Task leveraging such grading environments must support the SSH feature of INGInious.
TODO
TODO in further PRs
INGInious/base-containers/base/inginious_container_api/utils.py
Line 79 in a9962ca
Such logs could be used in further grading scripts.