Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Synapse Docker container doesn't start due to prometheus_client version 0.9.0 #8766

Closed
anoadragon453 opened this issue Nov 16, 2020 · 11 comments · Fixed by #8767
Closed

Synapse Docker container doesn't start due to prometheus_client version 0.9.0 #8766

anoadragon453 opened this issue Nov 16, 2020 · 11 comments · Fixed by #8767
Labels
A-Docker Docker images, or making it easier to run Synapse in a container. z-bug (Deprecated Label)

Comments

@anoadragon453
Copy link
Member

anoadragon453 commented Nov 16, 2020

Version v0.9.0 of the prometheus_client python module was just released, and now our docker containers fail with the following:

❯ docker run --name synapse \                                                                                                                                                                                        
    --mount type=volume,src=synapse-data,dst=/data \
    -p 8008:8008 \
    matrixdotorg/synapse:latest
ERROR:root:Needed prometheus_client>=0.4.0,<0.9.0, got prometheus-client==0.9.0
Missing Requirements: 'prometheus_client>=0.4.0,<0.9.0'
To install run:
    pip install --upgrade --force 'prometheus_client>=0.4.0,<0.9.0'
❯ 

We need to investigate why the docker container is installing a version of a module outside of what's specified in python_dependencies.py.

Looks like this is due to us installing the library separately without consulting the version requirements:

# Build dependencies that are not available as wheels, to speed up rebuilds
RUN pip install --prefix="/install" --no-warn-script-location \
frozendict \
jaeger-client \
opentracing \
prometheus-client \
psycopg2 \
pycparser \
pyrsistent \
pyyaml \
simplejson \
threadloop \
thrift

Currently anyone that builds a Synapse container will be affected, but our release containers built earlier should be fine.

@anoadragon453 anoadragon453 added z-bug (Deprecated Label) A-Docker Docker images, or making it easier to run Synapse in a container. p1 labels Nov 16, 2020
@anoadragon453
Copy link
Member Author

anoadragon453 commented Nov 16, 2020

A quick fix here is to add the version constraints to prometheus-client in this list, but a longer term fix should somehow apply the version constraints to the items in this list.

@anoadragon453
Copy link
Member Author

While we're in the area, we may want to make sure that all the dependencies here are actually still used.

@ddevault
Copy link

Did anyone consider, I dunno, addressing the breaking changes in Synapse and upgrading to the latest version of prometheus client?

anoadragon453 added a commit that referenced this issue Nov 17, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
)

Short-term fix for #8766.
@anoadragon453
Copy link
Member Author

@ddevault this was brought up in the linked PR: #8767 (comment)

@ddevault
Copy link

You should not institutionalize a habit of pinning your dependencies as a permanent solution. You should always be aiming to support the latest version of them, and ingrain this at a cultural level.

You're in for a world of hurt if you don't. When you're 6 versions behind on something, and suddenly it matters very much that you be on the latest version, but you have punted for long enough that you've ballooned your work 10x in the middle of a crisis.

@ddevault
Copy link

Personally, I would not have even merged a pull request like #8767 unless the submitter outlined a plan for upgrading and justified the temporary version pinning approach.

@callahad
Copy link
Contributor

You should not institutionalize a habit of pinning your dependencies as a permanent solution. You should always be aiming to support the latest version of them, and ingrain this at a cultural level. You're in for a world of hurt if you don't.

Acknowledged and agreed; we will not treat this as a permanent solution. There's some debt to unravel here, but we'll get there.

@amalon
Copy link

amalon commented Dec 2, 2020

This now breaks the archlinux package, since python-prometheus_client has updated to 0.9.0 so synapse won't start since its too new.

Oddly when I downgrade prometheus_client to 0.8.0-2 it doesn't detect prometheus_client at all... still trying to figure that one out.

@clokep
Copy link
Member

clokep commented Dec 2, 2020

I think this can be closed with the follow-up of #8831 filed. @anoadragon453 does that sound correct?

@ghost
Copy link

ghost commented Dec 3, 2020

@amalon Yep, I just broke my synapse install on Arch too. A simple hack to get up and running again until this is fixed upstream is to change /usr/lib/python3.9/site-packages/synapse/python_dependencies.py. Just remove the upper pin on "prometheus_client". Synapse will then start again, so it looks like 0.9.0 didn't break anything.

@anoadragon453
Copy link
Member Author

@clokep Indeed this issue pertaining to the Docker container can be closed. Discussion will continue in #8831 for removing the upper pin and ensuring everything still works.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Docker Docker images, or making it easier to run Synapse in a container. z-bug (Deprecated Label)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants