Skip to content

Refactor Docker Images, Evaluator Images Simple CI#823

Closed
JersyJ wants to merge 34 commits intomrlvsb:masterfrom
JersyJ:evaluator-refactor-images
Closed

Refactor Docker Images, Evaluator Images Simple CI#823
JersyJ wants to merge 34 commits intomrlvsb:masterfrom
JersyJ:evaluator-refactor-images

Conversation

@JersyJ
Copy link
Contributor

@JersyJ JersyJ commented Feb 7, 2026

  • Evaluator Docker Image Docker deployment #503
  • Docker Compose Evaluator Scheduler service definition - configuration with scheduler (1 rq worker)
  • Docker Compose Evaluator CPU Workers service definition - configuration with CPU workers (32 by default , EVALUATOR_CPU_REPLICAS)
  • Docker Compose Evaluator GPU Workers service definition - configuration with GPU workers (32 by default, EVALUATOR_CUDA_REPLICAS)
    EVALUATOR_REDIS__HOST
    EVALUATOR_REDIS__PORT
  • Reworked Evaluator Docker images (base/run/java/dotnet/clang-tidy/cargo/pythonrun)
  • Kelvin: Updated pipeline configuration/validation to allow selecting a Docker image for the "tests" type (pipe).
  • CI for Evaluator Docker Images with build.py (Unable to use GitHub Action Cache due to docker-container driver design/architecture (Cannot give a local image to FROM when using docker-container moby/buildkit#2343), reverting to local docker driver for compatibility.)
  • Update CI to use native GitHub Actions Cache instead of the Registry cache
  • Ruff format with Python 3.12 f-string compatibility
  • Document undocumented environment variable and add logic for running Evaluator inside container (API_INTERNAL_BASEURL)
  • Updated pyproject.toml dev dependency group (removed deprecated syntax)
  • Migrate from pre-commit to Rust re-implementation called prek
  • Update documentation and fix instructions that no longer works

@JersyJ JersyJ marked this pull request as ready for review February 8, 2026 15:17
Copilot AI review requested due to automatic review settings February 8, 2026 15:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors Kelvin’s Docker image setup (app + evaluator images) and CI workflows, while also updating developer tooling (switching from pre-commit to prek) and improving evaluator worker runtime behavior for Docker-based deployments.

Changes:

  • Reworked multiple Docker images (base/run/java/dotnet/clang-tidy/cargo/pythonrun) and added an evaluator worker image + build script.
  • Updated pipeline configuration/validation to allow selecting a Docker image for the tests pipe.
  • Updated CI caching strategy and developer tooling/docs (uv dependency groups, prek hooks).

Reviewed changes

Copilot reviewed 30 out of 31 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
web/views/student.py String formatting style adjustments.
uv.lock Removes pre-commit deps, adds prek, updates locked metadata.
pyproject.toml Migrates dev deps to [dependency-groups], adds prek.
frontend/src/PipelineValidation.js Adds tests.image validation + description.
evaluator/testsets.py Adjusts warning f-string quoting.
evaluator/pipelines.py Adds configurable tests image and changes docker ulimit formatting.
evaluator/images/run/Dockerfile Refactors apt install (leaner image, cleanup).
evaluator/images/pythonrun/Dockerfile Switches base + pins pytest/flake8.
evaluator/images/java/entry.py Removes runtime env-variable injection logic.
evaluator/images/java/Dockerfile Refactors installs and sets JAVA/Maven env via ENV.
evaluator/images/gcc/entry.py Adjusts PATH f-string quoting.
evaluator/images/gcc/Dockerfile Minor formatting cleanup.
evaluator/images/dotnet/Dockerfile Uses kelvin/base:latest, refactors apt install, adds env workaround.
evaluator/images/clang-tidy/Dockerfile Switches from Alpine to Debian-based kelvin/gcc and installs tools via apt.
evaluator/images/cargo/entry.py Adjusts warning string formatting.
evaluator/images/cargo/Dockerfile Refactors apt install, bumps bleach.
evaluator/images/build.py Major refactor into an image dependency scanner/builder CLI.
evaluator/images/base/Dockerfile Consolidates base deps, locale env, and bleach install.
evaluator/Dockerfile Adds a dedicated evaluator worker image that installs Docker CLI/engine packages.
docs/docs/01-intro/01-installation.mdx Updates installation steps for prek + worker command.
docker-compose.yml Adds internal base URL env, nginx aliasing, and evaluator worker services.
common/utils.py Adds API_INTERNAL_BASEURL handling with DEBUG guard.
common/event_log.py Fixes f-string quoting in __str__.
common/evaluate.py Adds DEBUG TLS verify override and ensures temp dir is under system temp /tmp/kelvin.
api/views/default.py Adjusts error f-string quoting.
Dockerfile Reworks build/runtime stages, uv installation method, runtime deps, and user/group creation.
.pre-commit-config.yaml Updates hook versions and consolidates ruff hooks.
.github/workflows/ci.yml Uses job defaults for deployment_service; switches Docker cache to GHA.
.github/workflows/build-evaluator-images.yml Adds workflow to build evaluator images when relevant files change.
.env.example Documents API_INTERNAL_BASEURL and evaluator worker envs.
.dockerignore Expands ignore patterns for Python/Node/editor artifacts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…tation, and change default image in TestsPipe
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 31 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JersyJ JersyJ force-pushed the evaluator-refactor-images branch from 498b373 to 0199ceb Compare February 9, 2026 00:46
Copy link
Collaborator

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Please separate a PR that only changes the CI, let's start with that.

# On PRs, we need everything to be green, while deploy jobs are skipped.
# On master, we need everything to be green.
# ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB!
conclusion:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The conclusion job won't work like this, I think I wrote it to you before. We either have to merge the two workflows, or use a different name for the conclusion job here (e.g. conclusion-images), so that we can configure CI to wait for both jobs to be green.

on:
pull_request:
merge_group:
workflow_dispatch:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The main issue that we have with the images is not that they break after we change them, but that they break when something external changes, most often apt repositories. So it would be great to run CI periodically to detect that sooner.

One way of doing that is running them always in CI, without file change detection. That has the annoying property that it can break CI for unrelated PRs. Another possibility is to setup a cron, to run this e.g. once a week. I'd go with the cron for now (in addition to the existing triggers).

uses: docker/build-push-action@v6
with:
cache-from: type=registry,ref=ghcr.io/mrlvsb/kelvin-ci-cache
cache-from: type=gha
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was the switch made?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to maintenance of the registry storage (LRU is used there automatically) and also the readability/visibility of that registry. Also this is official way and recommendation from GitHub and Docker.

@JersyJ JersyJ marked this pull request as draft February 9, 2026 14:00
@JersyJ
Copy link
Contributor Author

JersyJ commented Feb 9, 2026

Closing due to #824 #825 #826 #827

@JersyJ JersyJ closed this Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants