-
-
Notifications
You must be signed in to change notification settings - Fork 18
Update worker
Dockerfile
image to bookworm
#460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The issue here was that `slim-buster` has reached LTS and so trying to run `apt-get` commands on the image was returning 404 errors. The real robust solution is to upgrade to a newer future- proof image like `bookworm` instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pytest_celery/vendors/worker/Dockerfile (1)
7-21
: Shrink image size & avoid dangling layers.
apt-get update && apt-get install -y …
leaves APT caches in the layer and pulls in recommended packages by default, inflating the image >200 MB.-RUN apt-get update && apt-get install -y build-essential \ +RUN set -eux; \ + apt-get update; \ + apt-get install -y --no-install-recommends build-essential \ git \ @@ - sudo + sudo \ + && apt-get clean \ + && rm -rf /var/lib/apt/lists/*Benefits: ~80 MB smaller image, fewer CVE surfaces.
No functional change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pytest_celery/vendors/worker/Dockerfile
(1 hunks)
🔇 Additional comments (1)
src/pytest_celery/vendors/worker/Dockerfile (1)
1-1
: All dependencies exist in Debian Bookworm
Verified against packages.debian.org/bookworm—build-essential, git, wget, make, curl, apt-utils, debconf, lsb-release, libmemcached-dev, libffi-dev, ca-certificates, pypy3, pypy3-lib, and sudo all return HTTP 200. No renames or backports needed; you can safely bump topython:3.10-slim-bookworm
.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #460 +/- ##
=======================================
Coverage 23.80% 23.80%
=======================================
Files 41 41
Lines 1294 1294
Branches 94 94
=======================================
Hits 308 308
Misses 959 959
Partials 27 27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about updating to python 3.12 as well?
I'll try updating to |
Any chance this can be fixed soon? |
@Nusnus do you know what these CI failures are about? It looks like the existing issue, is there any chance there is caching affecting the testing suites that we can expire ? |
I think this works as a workaround for now:
FROM python:3.12-slim-bookworm
# Create a user to run the worker
RUN adduser --disabled-password --gecos "" test_user
# Install system dependencies
RUN apt-get update && apt-get install -y build-essential \
git \
wget \
make \
curl \
apt-utils \
debconf \
lsb-release \
libmemcached-dev \
libffi-dev \
ca-certificates \
pypy3 \
pypy3-lib \
sudo
# Set arguments
ARG CELERY_VERSION=""
ARG CELERY_LOG_LEVEL=INFO
ARG CELERY_WORKER_NAME=celery_test_worker
ARG CELERY_WORKER_QUEUE=celery
ENV WORKER_VERSION=$CELERY_VERSION
ENV LOG_LEVEL=$CELERY_LOG_LEVEL
ENV WORKER_NAME=$CELERY_WORKER_NAME
ENV WORKER_QUEUE=$CELERY_WORKER_QUEUE
ENV PYTHONUNBUFFERED=1
ENV PYTHONDONTWRITEBYTECODE=1
EXPOSE 5678
# Install Python dependencies
RUN pip install --no-cache-dir --upgrade \
pip \
celery[redis,pymemcache,gevent]${WORKER_VERSION:+==$WORKER_VERSION} \
pytest-celery[sqs]@git+https://github.com/celery/pytest-celery.git
# The workdir must be /app
WORKDIR /app
COPY content/ .
# Switch to the test_user
USER test_user
# Start the celery worker
CMD celery -A app worker --loglevel=$LOG_LEVEL -n $WORKER_NAME@%h -Q $WORKER_QUEUE
@pytest.fixture(scope="session")
def celery_base_worker_image(
docker_client,
default_worker_celery_version,
default_worker_celery_log_level,
default_worker_celery_worker_name,
default_worker_celery_worker_queue,
):
from pkg_resources import resource_filename
original_context_path = resource_filename("pytest_celery.vendors.worker", "")
custom_dockerfile = "Dockerfile"
custom_dockerfile_absolute_path = os.path.abspath(custom_dockerfile)
with open(custom_dockerfile, "rb") as f:
image, _ = docker_client.images.build(
path=original_context_path,
dockerfile=custom_dockerfile_absolute_path,
tag="pytest-celery/components/worker:default-custom",
buildargs={
"CELERY_VERSION": default_worker_celery_version,
"CELERY_LOG_LEVEL": default_worker_celery_log_level,
"CELERY_WORKER_NAME": default_worker_celery_worker_name,
"CELERY_WORKER_QUEUE": default_worker_celery_worker_queue,
},
rm=True,
)
return image |
1. We’re capped at 3.10 due to the `custom_setup` tests which use Celery 4. 2. The examples CI is bugged as it does not use the current branch’s changes and should work once the PR is merged to `main` (if everything else passes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The custom_setup tests are using Celery 4 with Celery 5 in the same test setup (hybrid worker cluster), which uses the same Dockerfile with different version args, which limits the Dockerfile base python to 3.10.
The right fix will be to create a separate Dockerfile for the Celery 4 worker in the tests, to “free” the Celery 5 base python from Celery 4 issues with newer python versions.
Due to the original reason this PR was opened, I prefer to accept the 3.10 change and then create a new PR for the “right fix”.
Take note the examples CI is bugged and should pass after merge on main
if everything else passes in the CI. This is another technical debt to fix :)
![]() |
v1.2.1 is now released with the fix. |
Upgrade the image used for the
pytest_celery_worker
from thepython:3.10-slim-buster
to thepython:3.10-slim-bookworm
image, because thebuster
images are on Debian 10, and it has already left the LTS, so therefore its package mirrors were moved from deb.debian.org to archive.debian.org.Once that happened, apt-get update inside the image began getting 404 / Release-file-missing errors, and the next apt-get install bailed out with exit code 100.
To reproduce on the
main
branch run:You will see an output like this:
Then on my branch, you will see the same command run without error.
Summary by CodeRabbit