Merge upstream #6
Open
Conversation
This file contains 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
* Use Python venv to build domserver because Sphinx version should be 6.1.0 or higher to avoid a build issue. * Use PHP 8.x because it's default in Debian. * Use libcgoup2 instead of libcgroup1, which is no longer available.
Fix PHP version in the PHP timezone configuration script, used to access PHP's configuration directory.
Encountring the following errors with --cap-add=sys_admin inside the dj_make_chroot script; one inside the debootstrap, and the other somewhere inside the script itself:
...
W: Failure trying to run: chroot "/chroot/domjudge" mount -t proc proc /proc
W: See /chroot/domjudge/debootstrap/debootstrap.log for details
...
mount: /chroot/domjudge/proc: cannot mount proc read-only.
dmesg(1) may have more information after failed mount system call.
...
The /chroot/domjudge/debootstrap/debootstrap.log does not exist nor the dmesg to investigate further (naive approach OFC).
Make the image slightly smaller by removing the older PHP versions. Start using the latest version for faster PHP. In domjudge/domjudge CI we use the lowest supported one so we catch the whole spectrum.
As mentiond by @christopher.
We fail on an error with libc-bin: https://gitlab.com/DOMjudge/domjudge-packaging/-/jobs/5876287000 I got this working in GitHub Actions already so we disable this here and fix this in GHA.
The default for PHP changes to 8.1. The package php8.1-json is now already provided
by both php8.1-{fpm,cli} and became virtual.
The npm pa11y tool is now install globally as npm changed its working,
given that we run this in CI as either domjudge or root having it
globally is actually better.
Although we duplicate code now, the intent is so much easier to follow
We could speed this up with creating the amd64 image in parallel but the bottleneck is always the arm64 as GitHub doesn't seem to have arm runners available yet. We now create a PR image which can be tested before in case this is needed.
The PRs for changes to those scripts will be stored in the registry of the user/organisation which forked or in our GitHub docker registry if this branch is under the domjudge organization. Here we always build against our latest version. The GitLab code had the option to not push the latest tag, for when we rebuild an older container, otherwise we always release against the overwritten value or if nothing was provided against the latest released tag (so which latest points to). The code for world readable files has been kept. Our build script is extended to now also have an option to push to another organization/namespace so we can push the image to the github container registry of the person doing the PR. As we don't do this often we explicit clean the github runner of older versions to make sure we always build against the latest image available of our dependencies and don't encounter the earlier builds if a PR is done more often (to fix something for example). The image can be locally tested by looking at the special tag based on the branchname/issue_number.
The repo was not properly quoted and the github.ref has another format with direct push. The push should be done without the tag after the image. Push resulting image to our DOMjudge GitHub container registry Alternative is to push to the own doing the PR, but they would be able to push another image to have the risk that in theory we test another PR than was used in the code from the PR. It seems to try to push the latest tag so make which tag pushed explicit.
- PRs/branches: ghcr - merged: DockerHub
We would trigger both on push and pull_request, skip the 2nd one. So we either run when this is a push in our organisation but not to main, OR if this is a pull_request from another organisation/user to domjudge_org.
When we push to our own organization this would trigger. As we only care for the push target in our own repo this is much easier. If someone would for they would need to change this but that is up to them.
This broke when the container user changed from root -> domjudge and not all actions as root were prefixed with sudo.
The branchname would become something hard to read and the branchname chosen by the contributor should already have an image as we work from a PR.
Building the image for the readonly branch is not needed as we already know this should work in the PR, only if someone would force merging before the CI passes we would need this.
Include an example which sets up a Traefik reverse proxy in Docker, including ACME for automated https certificate management. The deprecated and legacy `--link` flag of run commands is replaced with Docker networks in all run commands.
In the past we would search for the last pushed which would with semantic versioning be the same, this makes more sense IMHO.
We can't use latest as there is no release on domjudge.org with that name, but this makes atleast clear which index we try to get. This reverts commit 43982ab and improves on it.
We didn't install that version after the last upgrade.
This means we published a gitlabci container 22.04 which was actually 24.04.
The current build fails and the log is very hard to read
Moving this out of the security scope of the repository would make that we need to store this for the `github.author`. As we never used this before it's now taken out.
The workflows did not always trigger for PRs versus normal pushes.
Fixes: DOMjudge#193 I wonder why we never had this in the past as we do intent to create the domjudge cgroup in that directory.
The newer 'apt' command is recommended for interactive use, but not
for scripts. Running 'apt' non-interactively produces the following
message:
WARNING: apt does not have a stable CLI interface. Use with caution in scripts.
That package is already installed earlier in the same file.
It was removed as part of commit 7c8d5b9 (Upgrade gitlab image to 24.04, 2024-04-28), but it should be kept to reduce the size of the image (if the lists aren't removed in the same RUN directive as `apt-get update` then they will be stored in the layer and make the image larger, even if a later RUN directive removes the lists).
We already do this in all other places. It reduces the size of the image. (Note that the 'rm' command has to be done in the same RUN instruction as 'apt-get update', otherwise the lists will still be stored in the intermediate layers. So some of the RUN instructions had to be merged.)
- Move usage check to the top, to serve as documentation for people who read the script. - Remove `-x` from shebang line (#!), to only enable tracing in CI. - Fix `set -x` call (previously it was inside the `if` condition, which is wrong). - Remove PS4 variable (which adds info to the trace) because sh does not support LINENO, and the rest of the info is not that useful. - Inline `trace_off` into section_start and section_end to produce less noise in the trace. (The "section_start" lines will still be displayed though. Hiding them is tricky; see DOMjudge/domjudge/gitlab/integration.sh for an example, but note that it requires bash for `shopt -s expand_aliases`.) - Make the no-op placeholders for section_start and section_end produce less noise in the trace. - Redirect stderr to stdout as a workaround for a GitHub Actions issue that causes them to appear out-of-order. - Simplify initialisation of NAMESPACE variable to make the trace look nicer. - Put the variable assignments in a log group to make it look nicer. - Fix the invocation of build.sh in the build-domjudge-container-* workflows to use `./build.sh` rather than `sh ./build.sh` so that the options in the shebang line will be respected. Also remove unnecessary calls to `set -x` in the workflows.
Commit 62889e1 (Add instructions for setting up Traefik in Docker, 2024-02-06) switched from the legacy `--link` option to a user-defined bridge network (`--net`).
This is what the manual now suggests, and it's more consistent with dj_make_chroot which uses default-jre-headless. Not updating debian/control and live-image/install.sh for now because they are legacy or currently untested.
Previously, the DOMjudge Docker scripts changed the ownership of /opt/domjudge to "domjudge" recursively, overriding the ownership set by the DOMjudge installation commands (`make install-domserver` and `make install-judgehost`), which mostly set the owner to "root". It is unclear why the Docker scripts did that, since the DOMjudge installation commands should be responsible for installing with the correct ownership. This commit removes the `chown -R` calls from the Docker scripts in order to preserve the ownership set by the DOMjudge installation commands and avoid security issues. Note that the new behaviour is slightly fragile because it relies on Docker's `COPY --from` directive to preserve the ownership when copying files between build stages, and that only works if the numerical user and group IDs are the same. We plan to add a check that the IDs are the same.
We assume PHP8 already in the scripts, so I suspect this did not work anymore. When someone wants such an older version they can still check the history for the current files and alter the setup from there.
The CI broke as the cache only knows of an older version of Chrome.
Also actually cleanup the cache, there is another better method for that but I'll leave that for another PR.
Adding configuration for Apache2 enables easier testing of webserver-specific features and issues. By default, the contributor image still uses NGINX as webserver. Add an option to use Apache2 by default, or switch back and forth between NGINX/Apache2 with the `switch-webserver` command.
If the container is restarted, the configuration file does not exist. Do not fail the `rm` command if the file could not be removed to ensure idempotency of the Apache2 configuration part. Fixes an issue introduced in 6283816.
NOTE: Might need cgroupv2 apt package
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.
PR to merge the upstream of this fork in this repository again. This will hopefully eliminate (future) small errors in the building/running of DOMjudge Docker containers