Skip to content

Conversation

@nrybowski
Copy link
Member

@nrybowski nrybowski commented May 23, 2023

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

  • Patch virtme to enable port forwarding for telnet
  • Add a 'KVM' label in the base container to dynamically enable KVM passthrough on the Agents
  • Add 'KVM' flag to Docker Agent
    • Check if KVM is enabled on the Agent environment
  • Automate KVM launch in student container
  • Automate shell redirection to KVM in student container
  • Hardcode qemu-kvm src rpm url
  • Add documentation

TODO in further PRs

  • Use serial port to spawn a shell (see virtme) in the VM rather than using telnet and port forwarding
  • Log the commands typed in the telnet shell as here

https://github.com/UCL-INGI/INGInious/blob/a9962cad2a18caf87a73f10c627fc98f561f0e6c/base-containers/base/inginious_container_api/utils.py#L79

Such logs could be used in further grading scripts.

anaHue and others added 30 commits July 4, 2022 11:17
[Frontend/Submissions] fix - change class for file code icon
…ous#821)

* using tabs to simplify display

* fix tiny bug if "activate lti" is checked and then unchecked, it displays "access control type" independently of selected access control

* Hide lti tab if lti option is not activated in general tab
This keeps the file metadata (permissions, dates, ...). Fix INGInious#390
…nious#849)

Warning: this doesn't ensure retrocompatibilty with previous weighing system.
The new router does not perform correctly when using regex groups. Moreover,
it is now required to explicitely tell it the converter can match the forward /
using ``part_isolating``.
[backend] do not erase previous state in case of errors
Currently translated at 92.3% (680 of 736 strings)

Translation: INGInious/Frontend
Translate-URL: https://weblate.info.ucl.ac.be/projects/inginious/frontend/fr/
anthonygego and others added 8 commits April 20, 2023 11:03
[agent/docker] replace SSH opt-in by env types
Fixes INGInious#697.

This is a first mitigation and in all case useful, as the limit is set on the
total command sent to the MongoDB server, which cannot be computed before.
[frontend/submission_manager] catch DocumentTooLarge exception
Updated setup requirements to Flask 2.0+ that introduces the new
function.

Fix INGInious#942
nrybowski added a commit to nrybowski/INGInious-containers that referenced this pull request May 23, 2023
@nrybowski
Copy link
Member Author

This PR should solve #939.

@nrybowski nrybowski marked this pull request as ready for review May 25, 2023 23:24
@nrybowski
Copy link
Member Author

Some slowness is observed during the execution of a simple mininet command such as mn --switch lxbr --topo tree,depth=2,fanout=8 --test pingall at the startup of the fifth switch.
The VM has two cores and 256Mo of RAM but none of those ressources are used at their maximum.
Further investigation is required but do not prevent the integration of this feature which is anyway still experimental.

Copy link
Member

@anthonygego anthonygego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

runtime=runtime,
ulimits=[nofile_limit]
ulimits=[nofile_limit],
devices=['/dev/kvm'] if kvm and image_requires_kvm else []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

# Check the runtimes
runtime = sys.argv[1]
parent_runtime = sys.argv[2]
kvm_enabled = sys.argv[3] == "True"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The arguments managment will be replaced by an argparser.

@nrybowski nrybowski force-pushed the master branch 2 times, most recently from e53b0e2 to 9a6d192 Compare December 21, 2024 13:51
@nrybowski
Copy link
Member Author

v1.32 of virtme now supports SSH. We should bump the version and replace the telnet hack with this new feature.

@anthonygego anthonygego changed the base branch from master to main March 25, 2025 16:18
@anthonygego
Copy link
Member

Can you rebase your branch against main (you'll probably have to cherry-pick) ? I think there should be no or very few conflicts to solve.

@anthonygego anthonygego marked this pull request as draft April 11, 2025 15:25
@nrybowski nrybowski added the Feedback needed Project maintainers wait for feedback from OP label Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feedback needed Project maintainers wait for feedback from OP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runtime environments allowing to run as root could provide more capabilities to the containers