feat: migrate to uv + add ContainerGrader for Kubernetes/Docker sandboxed grading#14
Open
blarghmatey wants to merge 19 commits intomasterfrom
Open
feat: migrate to uv + add ContainerGrader for Kubernetes/Docker sandboxed grading#14blarghmatey wants to merge 19 commits intomasterfrom
blarghmatey wants to merge 19 commits intomasterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly modernizes the Highlights
Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modernizes the xqueue-watcher project by migrating to uv for package management and introducing a ContainerGrader for Kubernetes/Docker sandboxed grading, enhancing security and deployment flexibility. However, a high-severity path traversal vulnerability was identified in the grader path construction logic, which could allow arbitrary code execution or sensitive information disclosure. Strict validation on the grader path is recommended. Additionally, inconsistencies and areas for improvement were noted in the Dockerfile, documentation, and error handling that should be addressed.
- Run migrate-to-uv to bootstrap pyproject.toml from requirements/base.txt and requirements/test.txt - Add full project metadata: name, version, description, requires-python>=3.11, license, hatchling build backend, entry point xqueue-watcher -> manager:main - Add newrelic as [project.optional-dependencies.production] - Add dev dependency group: coverage, mock, pytest-cov - Remove setup.py (replaced by pyproject.toml) - Remove all requirements/*.in and requirements/*.txt files (14 files) - Generate uv.lock with pinned dependency graph - Update Makefile: replace pip/pip-compile targets with uv sync / uv run pytest - Update .github/workflows/ci.yml: use astral-sh/setup-uv@v4, drop ubuntu-20.04 and Python 3.8, add Python 3.13, update to actions/checkout@v4 - Replace upgrade-python-requirements workflow with uv lock --upgrade + create-pull-request workflow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove six (Python 2 compat shim) from imports and SUPPORT_FILES in jailedgrader.py — Python 3 only going forward - Wrap codejail imports in try/except in jailedgrader.py and manager.py; raise RuntimeError with clear message directing users to ContainerGrader - Fix Path.abspath() -> Path.absolute() (breaking API change in path v17) in grader.py and jailedgrader.py - Update Dockerfile: ubuntu:xenial -> python:3.11-slim, remove apparmor and language-pack-en packages, fix layer ordering - Update test_codejail_config to use fork_per_item=False to avoid multiprocessing state-inheritance failure on Python 3.14 forkserver default - Update conf.d/600.json example to use ContainerGrader handler Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds xqueue_watcher/containergrader.py — a drop-in replacement for
JailedGrader that executes student code inside an isolated container
instead of using AppArmor/codejail.
Security model (replaces AppArmor):
- Container isolation (Linux namespaces + cgroups)
- Non-root user (UID 1000), read-only root filesystem
- CPU/memory resource limits enforced by container runtime
- Network disabled for grader containers (no egress)
- Hard wall-clock timeout via activeDeadlineSeconds (k8s) or timeout (Docker)
Two pluggable backends selected via the 'backend' KWARGS option:
kubernetes (default / production):
- Creates a batch/v1 Job per submission using the kubernetes Python client
- Auto-detects in-cluster vs kubeconfig credentials
- Polls until Job completes, collects stdout from pod logs
- Deletes the Job after result collection (ttlSecondsAfterFinished=300)
- Job pod spec includes: securityContext, resource limits,
activeDeadlineSeconds, and labels for observability
docker (local dev / CI):
- Runs a container via the docker Python SDK
- Bind-mounts the grader directory read-only
- Passes SUBMISSION_CODE as an environment variable
- Network disabled, memory + CPU limits applied
Student code is passed via SUBMISSION_CODE env var (avoids argv length
limits and shell injection). The entrypoint writes it to /tmp before
invoking grader_support.run, producing the same JSON output format that
JailedGrader already expects — so no changes to grader test framework
or course team grader code are required.
Configuration example (conf.d/my-course.json):
{
"my-course": {
"HANDLERS": [{
"HANDLER": "xqueue_watcher.containergrader.ContainerGrader",
"KWARGS": {
"grader_root": "/graders/my-course/",
"image": "registry.example.com/my-course:latest",
"backend": "kubernetes",
"cpu_limit": "500m",
"memory_limit": "256Mi",
"timeout": 20
}
}]
}
}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
grader_support/Dockerfile.base:
- python:3.11-slim base, non-root grader user (UID 1000)
- Copies grader_support framework; installs path-py
- ENTRYPOINT: python -m grader_support.entrypoint
- /tmp volume for submission files (writable even with read-only root fs)
- Course teams extend this image to add their deps and grader scripts
grader_support/entrypoint.py:
- Reads SUBMISSION_CODE env var, writes to /tmp/submission.py
- Adds /tmp and cwd to sys.path, then delegates to grader_support.run
- Prints JSON result to stdout (same schema JailedGrader already parses)
grader_support/README.md:
- Course team authoring guide: how to extend the base image, configure
the handler, and understand the security properties
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
deploy/kubernetes/ (Kustomize-compatible):
- serviceaccount.yaml — dedicated SA for xqueue-watcher pods
- rbac.yaml — Role + RoleBinding: create/delete Jobs, read pod logs
- configmap.yaml — watcher xqwatcher.json config (edit for your queues)
- deployment.yaml — 2 replicas, topologySpreadConstraints, securityContext,
resource limits, readinessProbe
- networkpolicy.yaml — deny all ingress/egress on grader Job pods (label:
role=grader-job); allow xqueue-watcher egress to xqueue
- secret.yaml.template — placeholder: copy to secret.yaml, fill in credentials,
do not commit secret.yaml (added to .gitignore)
- kustomization.yaml — Kustomize entry point for the base directory
docker-compose.yml (local dev):
- xqueue-watcher container with docker socket access (for docker backend)
- Mounts conf.d/ and grader directories
- Includes a sample xqueue service reference for full local stack
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ContainerGrader had two bugs affecting how grader files were located inside the container at runtime: 1. Docker backend bind-mounted the grader problem directory at /grader, overwriting the grader_support package that the base image copies there. Fixed by binding at /graders instead and passing the resulting absolute in-container path (/graders/<file>) to the entrypoint. 2. Kubernetes backend set working_dir to the grader problem directory (e.g. /graders/ps07/Robot/), preventing Python from finding the grader_support package which lives at /grader/grader_support/. Fixed by keeping working_dir=/grader (the base image WORKDIR) and passing the absolute grader path in args instead of just the basename. entrypoint.py previously passed the full absolute path verbatim to __import__(), which fails for paths containing slashes. It now detects absolute paths, inserts the parent directory into sys.path, and uses only the basename as the importable module name. Also updates grader_support/README.md to document the correct layout (/graders/ for course grader scripts, /grader/ for grader_support) and the gradelib compatibility note for course teams migrating from Python 2 graders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
blarghmatey
force-pushed
the
chore/migrate-to-uv-and-k8s-container-grader
branch
from
d59ff3b to
18c7151
Compare
codejail is an optional dependency (not installed in CI). Guard the import with a try/except and apply @pytest.mark.skipif to the test class so collection succeeds and tests are skipped gracefully when codejail is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Dockerfile: replace deleted requirements/ pip install with uv sync (copies uv binary from ghcr.io/astral-sh/uv and uses uv sync --frozen) - grader.py: guard against path traversal in grader_config['grader']; validate that the resolved grader path stays within grader_root - containergrader.py: fix Docker SDK TypeError - containers.run() does not accept a timeout kwarg; switch to detach=True + container.wait() to enforce the timeout, then collect logs and remove the container - containergrader.py: remove brittle hardcoded line numbers (L364, L379, L397, L450) from user-facing error messages - docker-compose.yml: change conf.d and data volumes from :ro to :rw so local edits take effect without rebuild (matches comment intent) - upgrade-python-requirements.yml: add explicit permissions block (contents: write, pull-requests: write) as required by security policy - Automated code Graders With xqueue-watcher.md: remove empty heading, add 'Property' header to comparison table Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes xqueue-watcher’s grading execution and packaging by introducing a container-based grader (Docker/Kubernetes) as the preferred sandboxing mechanism, while making the legacy AppArmor/codejail path optional. It also migrates the project from pip-compile/requirements + setup.py to a pyproject.toml + uv.lock workflow and adds deployment scaffolding for local Docker Compose and Kubernetes.
Changes:
- Add
ContainerGrader(Docker + Kubernetes backends) plus container entrypoint/base image undergrader_support/. - Make codejail optional and improve error messaging when codejail is unavailable.
- Replace requirements/setup.py tooling with Hatch/
pyproject.toml,uv.lock, updated CI workflow(s), plus new local/K8s deployment manifests.
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| xqueue_watcher/containergrader.py | New container-based grading implementation (Docker + Kubernetes backends). |
| grader_support/entrypoint.py | Container entrypoint that writes submission code to /tmp and runs the framework runner. |
| grader_support/Dockerfile.base | Base grader image to run student code non-root with the entrypoint. |
| grader_support/README.md | Documentation for building/using the grader base image and handler configuration. |
| xqueue_watcher/jailedgrader.py | Treat codejail as optional; fail fast with a clearer error when absent; remove legacy six support-file wiring. |
| xqueue_watcher/manager.py | Make codejail import optional and raise an explicit error if configured without dependency installed. |
| xqueue_watcher/grader.py | Switch to Path.absolute() for grader path resolution. |
| tests/test_manager.py | Adjust codejail test to avoid relying on subprocess/fork inheritance behavior. |
| pyproject.toml | New project metadata and dependency definition for uv/hatch-based packaging. |
| uv.lock | New uv lockfile for dependency pinning. |
| Makefile | Reworked targets to use uv + docker-compose workflows. |
| Dockerfile | Updated runtime container build, user setup, and New Relic stage. |
| docker-compose.yml | New local dev stack including xqueue and watcher service. |
| deploy/kubernetes/* | New kustomize-ready manifests for service account, RBAC, deployment, configmap, and network policy. |
| conf.d/600.json | Update sample handler config to use ContainerGrader with docker backend. |
| .github/workflows/ci.yml | Switch CI to uv and expand Python test matrix. |
| .github/workflows/upgrade-python-requirements.yml | Replace requirements-bot workflow with uv lock --upgrade PR automation. |
| .gitignore | Ignore uv venv and local Kubernetes secret manifest. |
| setup.py | Removed legacy setuptools packaging entrypoint. |
| requirements/* | Removed pip-compile based requirements/constraints files (directory deleted). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
path-py is an external dependency that wraps pathlib with a fluent API.
Since we now require Python >= 3.11, pathlib covers all the same
functionality without an extra dependency.
Changes:
- Replace 'from path import Path' with 'from pathlib import Path' in all
source and test files
- .dirname() → .parent
- .basename() → .name
- .absolute() / .absolute() → .resolve() (symlink-safe)
- .files('*.json') → .glob('*.json') (with sorted() for stable ordering)
- Remove path-py (path-py / path) from pyproject.toml dependencies
- Regenerate uv.lock (removes path==17.1.1 and path-py==12.5.0)
- Simplify grader.py path-traversal check: now that grader_path is a
native pathlib.Path, the inline 'import pathlib' is no longer needed
- Fix test_grader.py mock: grader_path.endswith() → grader_path.name ==
- Fix test_manager.py: pass str() to argparse (Path is not subscriptable)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lation decision Add edx-codejail (the upstream PyPI package, v4.1.0) as an optional 'codejail' extra, replacing the previously pinned git-URL reference to a specific commit. uv add --optional codejail edx-codejail codejail is intentionally excluded from the base Docker image because ContainerGrader uses container-level isolation (Linux namespaces, cgroups, capability dropping, network isolation, read-only filesystem) which provides equivalent sandboxing to AppArmor without requiring host-level AppArmor configuration that is unavailable inside Kubernetes pods. Install the 'codejail' extra only when using the legacy JailedGrader on a bare-metal or VM host with AppArmor configured. To use: uv sync --extra codejail Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 43 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Makefile: fix tab indentation on all recipe lines (was space-indented) - grader.py: remove unused sys import - jailedgrader.py: replace deprecated load_module() with spec_from_file_location/exec_module - containergrader.py: - remove unused imports (logging, os, tempfile) and _JOB_LABEL constant - add emptyDir volume at /tmp in K8s Job spec (required when read_only_root_filesystem=True) - add clarifying comment that K8s grader scripts are baked into the course image - replace deprecated load_module() with importlib.util spec/exec_module pattern - capture stderr from Docker container on non-zero exit for better diagnostics - grader_support/entrypoint.py: correct misleading comment about /tmp writability - deploy/kubernetes/deployment.yaml: fix command to use xqueue-watcher entry point - deploy/kubernetes/configmap.yaml: add xqueue-watcher-queue-configs ConfigMap so manifests apply cleanly out of the box - docker-compose.yml: mount Docker socket for docker backend to work - conf.d/600.json: use absolute /graders/ path instead of relative ../data path - Dockerfile: use C.UTF-8 locale (available without installing locales package) - pyproject.toml: add edx-codejail to dev group so jailed grader tests run in CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/gemini review |
|
Warning Gemini is experiencing higher than usual traffic and was unable to create the review. Please try again in a few hours by commenting |
…der unit tests
Architecture change: grader scripts are baked into the course-specific Docker
image, so the watcher pod has no need to access grader files locally. The
grader_support entrypoint now runs the complete grading pipeline inside the
container (load grader, preprocess, run answer + submission, compare, return
JSON grade), and ContainerGrader.grade() is simplified to just launch the
container and parse its JSON output.
Changes:
- grader_support/entrypoint.py: complete rewrite; now takes GRADER_FILE SEED
(not GRADER_FILE submission.py SEED); runs full grade pipeline in container;
reads GRADER_LANGUAGE and HIDE_OUTPUT env vars from ContainerGrader
- xqueue_watcher/containergrader.py:
- Remove grader-module loading, gettext, answer.py reading, and all test-
comparison logic from grade() — the container handles this now
- grade() now just calls _run() and parses the returned JSON
- _run() accepts grader_config and forwards lang/hide_output as env vars
- _build_k8s_job(): args are now [grader_abs, seed] (not 3 args), adds
GRADER_LANGUAGE and HIDE_OUTPUT env vars, still mounts emptyDir at /tmp
- _run_docker(): same arg change; passes GRADER_LANGUAGE and HIDE_OUTPUT
- ReadTimeout from container.wait() caught and re-raised as clear RuntimeError
- Remove unused _truncate, _prepend_coding, importlib.util
- tests/test_container_grader.py: 36 new unit tests covering:
- _parse_cpu_millis
- ContainerGrader init / backend validation
- _build_k8s_job: args, env vars, resource limits, emptyDir/tmp, security
- _run_docker: success, non-zero exit (with stderr), timeout, missing SDK
- grade(): skip_grader, successful result, container failure, size warning
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 44 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 47 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…ettings - Remove dogstatsd-python dependency; replace statsd instrumentation in grader.py with OpenTelemetry counters and a histogram - Add xqueue_watcher/metrics.py: configure_metrics() wires a MeterProvider with an OTLP HTTP exporter when OTEL_EXPORTER_OTLP_ENDPOINT is set; all four instruments (process_item, grader_payload_error, grading_time, replies) defined at module level against the global proxy meter - Call configure_metrics() from Manager.configure_from_directory() so the real provider is installed before any submissions are processed - Add xqueue_watcher/env_settings.py: get_manager_config_from_env() reads all manager config from XQWATCHER_* environment variables, compatible with 12-factor / Kubernetes deployment patterns - Remove newrelic from the production optional-dependency group and from the edx.org Dockerfile stage; the stage now runs xqueue-watcher directly - Add opentelemetry-api, opentelemetry-sdk, opentelemetry-exporter-otlp-proto-http to core dependencies; regenerate uv.lock - Add tests/test_env_settings.py and tests/test_metrics.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- docker-compose.yml: remove unused GRADER_BACKEND env var, fix duplicate volumes key by merging into one list, tag sample-grader with image: grader-base:local so conf.d/600.json reference resolves - Dockerfile: standardise CMD config path to /etc/xqueue-watcher to match docker-compose and Kubernetes manifests - metrics.py: remove OTEL_METRIC_EXPORT_INTERVAL from docstring since it is not wired up in _build_meter_provider() - containergrader.py: add pod template metadata labels so the NetworkPolicy podSelector (app.kubernetes.io/component=xqueue-grader) actually matches grading pods; set automount_service_account_token=False on the grading pod spec to reduce blast radius if the NetworkPolicy is misconfigured; add _parse_memory_bytes() helper and use it for the Docker backend mem_limit so Kubernetes-style strings like '256Mi' are converted to bytes rather than passed raw (which Docker does not accept) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
blarghmatey
force-pushed
the
chore/migrate-to-uv-and-k8s-container-grader
branch
from
32d6ffc to
16e034b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 47 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| # Kubernetes pods and adds no meaningful security benefit on top of container | ||
| # isolation. Install the `codejail` extra only when running the legacy | ||
| # JailedGrader on a bare-metal or VM host with AppArmor configured. | ||
|
|
|
|
||
| RUN useradd -m --shell /bin/false app | ||
|
|
||
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv |
Comment on lines
+286
to
+312
| grader_dir = str(Path(grader_path).parent.resolve()) | ||
| grader_rel = str(Path(grader_path).name) | ||
| # Mount the problem directory at /graders/ (not /grader/ which would | ||
| # overwrite the base image's grader_support package). Pass the grader | ||
| # as an absolute in-container path. | ||
| container_grader_path = f"/graders/{grader_rel}" | ||
|
|
||
| env = { | ||
| "SUBMISSION_CODE": code, | ||
| "GRADER_LANGUAGE": grader_config.get("lang", "en"), | ||
| "HIDE_OUTPUT": "1" if grader_config.get("hide_output") else "0", | ||
| } | ||
|
|
||
| client = docker_sdk.from_env() | ||
| try: | ||
| # Run detached so we can enforce a wall-clock timeout via container.wait(). | ||
| # containers.run() does not accept a timeout argument; using detach=True | ||
| # lets us call container.wait(timeout=...) to cap execution time. | ||
| container = client.containers.run( | ||
| image=self.image, | ||
| # entrypoint signature: GRADER_FILE SEED | ||
| command=[container_grader_path, str(seed)], | ||
| working_dir="/grader", | ||
| environment=env, | ||
| volumes={grader_dir: {"bind": "/graders", "mode": "ro"}}, | ||
| mem_limit=_parse_memory_bytes(self.memory_limit), | ||
| nano_cpus=int(_parse_cpu_millis(self.cpu_limit) * 1_000_000), |
Comment on lines
+25
to
+29
| volumes: | ||
| # Mount the local conf.d so you can edit queue configs without rebuilding. | ||
| - ./conf.d:/etc/xqueue-watcher/conf.d:rw | ||
| # Mount local grader scripts for rapid iteration. | ||
| - ./data:/graders:rw |
Comment on lines
+8
to
+10
| # Install minimal runtime dependency needed by grader_support | ||
| RUN pip install --no-cache-dir path-py | ||
|
|
Comment on lines
+145
to
+147
| elapsed = time.time() - start | ||
| _metrics.grading_time_histogram.record(elapsed) | ||
| self.log.info('grading-time seconds=%.3f', elapsed) |
Comment on lines
+9
to
+12
| def test_defaults_when_no_env_vars_set(self): | ||
| with patch.dict("os.environ", {}, clear=False): | ||
| config = get_manager_config_from_env() | ||
| self.assertEqual(config, MANAGER_CONFIG_DEFAULTS) |
Comment on lines
+58
to
+61
| def test_follow_client_redirects_default_is_false(self): | ||
| with patch.dict("os.environ", {}, clear=False): | ||
| config = get_manager_config_from_env() | ||
| self.assertFalse(config["FOLLOW_CLIENT_REDIRECTS"]) |
Comment on lines
+14
to
+18
| class TestBuildMeterProvider(unittest.TestCase): | ||
| def test_returns_meter_provider(self): | ||
| with patch.dict("os.environ", {}, clear=False): | ||
| provider = _build_meter_provider() | ||
| self.assertIsInstance(provider, MeterProvider) |
uv installs the console script into the project virtual environment at .venv/bin/xqueue-watcher. Without adding this directory to PATH the CMD cannot be found at container startup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When no logging.json file is present, manager.py now calls configure_logging() from env_settings instead of basicConfig(). configure_logging() sets up a single StreamHandler on stdout with a consistent timestamp/level/module format, honours XQWATCHER_LOG_LEVEL (default INFO), and suppresses noisy requests/urllib3 debug output. This removes the need for a logging.json file in Kubernetes deployments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Using PATH via ENV is fragile -- container runtimes and security policies can reset or ignore it. Install a symlink at /usr/local/bin/xqueue-watcher (always in the standard system PATH) so the entrypoint resolves regardless of how the container is launched. Also remove the stale NEW_RELIC_LICENSE_KEY env entry from the Kubernetes deployment manifest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+142
to
+143
| if e is not None: | ||
| act_out += f"\n*** ERROR: {e} ***" |
There was a problem hiding this comment.
Bug: The condition if e is not None: inside an except EndTest as e: block is always true, causing an empty error message to be shown to students.
Severity: MEDIUM
Suggested Fix
Replace the check if e is not None: with a check on the exception's message content, such as if str(e): or if e.args:. This will correctly verify whether the EndTest exception was raised with a message before appending anything to the output.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: grader_support/entrypoint.py#L142-L143
Potential issue: In an `except EndTest as e:` block, the exception object `e` can never
be `None`. However, the code checks `if e is not None:`, which is always true. This
causes an error message string, `"\n*** ERROR: {e} ***"`, to be appended to the output
even when the `EndTest` exception is raised without a message. As a result, when a test
ends via `EndTest()`, students will see a confusing, empty error message like `"***
ERROR: ***"` in their output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR modernizes xqueue-watcher on two fronts:
Why this change?
The existing
JailedGraderrelies oncodejail, which requires AppArmor kernel profiles and OS-level user-switching (setuid). This makes it incompatible with standard Kubernetes deployments. The existing base image (ubuntu:xenial) is also end-of-life.See
Automated code Graders With xqueue-watcher.mdfor prior art and context.What changed
uv migration (
chore:commit)setup.py+ allrequirements/*.{in,txt}(14 files) withpyproject.toml+uv.lock[production]extra (newrelic), and dev dependency groupMakefilewithuvtargetsastral-sh/setup-uv@v4, Python 3.11/3.12/3.13, ubuntu-24.04uv lock --upgrade+ create-pull-requestCompatibility fixes (
fix:commit)pathv17 renamedabspath()→absolute()— fixed ingrader.pyandjailedgrader.pycodejailimport optional (try/except) injailedgrader.pyandmanager.py; raisesRuntimeErrorpointing to ContainerGrader if not installedsix(Python 2 shim) entirelyDockerfile:ubuntu:xenial→python:3.11-slim, removed apparmor packagestest_codejail_config: addedfork_per_item=Falsefor Python 3.14 forkserver compatibilityContainerGrader (
feat:commit)New
xqueue_watcher/containergrader.py— drop-in replacement forJailedGrader:grade(grader_path, grader_config, submission)interfacekubernetesbackend (production): creates abatch/v1Job per submission; collects stdout from pod logs; deletes Job after completiondockerbackend (local dev): runs container via Docker SDK; bind-mounts grader directoryactiveDeadlineSeconds, network disabled for grader containers{ "my-course": { "HANDLERS": [{ "HANDLER": "xqueue_watcher.containergrader.ContainerGrader", "KWARGS": { "grader_root": "/graders/my-course/", "image": "registry.example.com/my-course:latest", "backend": "kubernetes", "cpu_limit": "500m", "memory_limit": "256Mi", "timeout": 20 } }] } }Grader base image (
feat:commit)grader_support/Dockerfile.base: base image course teams extendgrader_support/entrypoint.py: readsSUBMISSION_CODEenv var →/tmp/submission.py→ invokesgrader_support.rungrader_support/README.md: course team authoring guideKubernetes manifests (
feat:commit)New
deploy/kubernetes/directory (Kustomize-compatible):serviceaccount.yaml,rbac.yaml— create/delete Jobs, read pod logsdeployment.yaml— 2 replicas, securityContext, resource limits, readinessProbenetworkpolicy.yaml— deny all ingress/egress on grader Job pods; allow watcher→xqueueconfigmap.yaml,secret.yaml.template,kustomization.yamldocker-compose.ymlfor local full-stack developmentWhat is NOT changing
client.py,manager.py)grader_support/gradelib.py,run.py)Security model comparison
--network noneactiveDeadlineSeconds/timeoutTesting
All 25 existing tests pass (
uv run pytest). ContainerGrader tests require a live Docker/k8s environment and are integration tests; unit tests for the class itself can be added in a follow-up PR.