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

Merge the Complement testing Docker images into a single, multi-purpose image. #12881

Merged
merged 28 commits into from
Jun 8, 2022
Merged

Merge the Complement testing Docker images into a single, multi-purpose image. #12881

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e4137c1
Mutiny: oust the monolith image and replace with the workers image
reivilibre May 24, 2022
865fa26
Update references in Dockerfile
reivilibre May 24, 2022
edc7fcd
Update complement.sh to use the unified image
reivilibre May 24, 2022
15215bd
Make it possible to disable Redis with an env var
reivilibre May 26, 2022
97699e3
Require START_POSTGRES=1 to start Postgres
reivilibre May 26, 2022
9cd96d2
Make the Complement Synapse launcher able to handle all 3 scenarios
reivilibre May 26, 2022
3a588a0
Don't enable Redis for the monolith case
reivilibre May 26, 2022
7bc03a6
Burn env vars into the image (sad and hopefully temporary)
reivilibre May 26, 2022
bf37bf3
Newsfile
reivilibre May 26, 2022
facccd0
Bring back the experimental features
reivilibre May 26, 2022
7e9884a
Apply suggestions from Rich
reivilibre May 30, 2022
423e212
Remove mention of 'worker' from entrypoint
reivilibre May 30, 2022
470fabe
Pass the environment variables straight through, with no burning!
reivilibre May 30, 2022
b0241e7
Add back SYNAPSE_VERSION build arguments
reivilibre May 30, 2022
3537762
Automatically disable Redis only for no-worker instances
reivilibre May 31, 2022
8ae3da7
Improve Complement documentation about 'Stuff' :)
reivilibre May 31, 2022
33aeb67
Update the contributing guide
reivilibre May 31, 2022
9f8d317
Update README-testing.md
reivilibre May 31, 2022
9531da6
Fix sentence that dropped off
reivilibre May 31, 2022
2beae8f
Update docker/README-testing.md
reivilibre May 31, 2022
bc5d529
Apply suggestions from code review
reivilibre May 31, 2022
166b046
Remove obsolete START_REDIS
reivilibre May 31, 2022
4a739ca
Fix comment explaining the overwritten config file
reivilibre May 31, 2022
1af38ef
Convert workers-shared.yaml to Jinja2 and import the original file ra…
reivilibre Jun 1, 2022
6daac8a
Add comment about how it fits in
reivilibre Jun 6, 2022
6c01955
Merge branch 'develop' into rei/one_complement_image
reivilibre Jun 6, 2022
2fd415b
Update docker/configure_workers_and_start.py
reivilibre Jun 8, 2022
b548132
Move comment to top of file and rephrase slightly
reivilibre Jun 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 32 additions & 14 deletions docker/complement/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,40 @@
# A dockerfile which builds an image suitable for testing Synapse under
# complement.
# This dockerfile builds on top of 'docker/Dockerfile-worker' in matrix-org/synapse
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
# by including a built-in postgres instance, as well as setting up the homeserver so
# that it is ready for testing via Complement.
#
# Instructions for building this image from those it depends on is detailed in this guide:
# https://github.com/matrix-org/synapse/blob/develop/docker/README-testing.md#testing-with-postgresql-and-single-or-multi-process-synapse
FROM matrixdotorg/synapse-workers
Copy link
Member

Choose a reason for hiding this comment

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

the ability to run Complement against various versions of Synapse by changing the SYNAPSE_VERSION build param is something I've used from time to time to bisect where a problem landed, and it would be quite nice to retain that ability, presumably by parameterising docker/Dockerfile-workers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks — I should be able to support that easily.

Out of interest: Does using git bisect/checking out the relevant version of the source tree and using complement.sh not do what you want?

Copy link
Member

Choose a reason for hiding this comment

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

sure it does, at the expense of building the entire Synapse image from scratch for each version ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're running it with the pre-builts? Yeah, that sounds like it'd be good to support

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will note just in case you think it would be useful: we could revive #11852 to publish the synapse-worker images to GHCR if that would make this kind of investigation easier.


# Install postgresql
RUN apt-get update && \
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y postgresql-13

# Configure a user and create a database for Synapse
RUN pg_ctlcluster 13 main start && su postgres -c "echo \
\"ALTER USER postgres PASSWORD 'somesecret'; \
CREATE DATABASE synapse \
ENCODING 'UTF8' \
LC_COLLATE='C' \
LC_CTYPE='C' \
template=template0;\" | psql" && pg_ctlcluster 13 main stop

# Modify the shared homeserver config with postgres support, certificate setup
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
# and the disabling of rate-limiting
COPY conf-workers/workers-shared.yaml /conf/workers/shared.yaml

ARG SYNAPSE_VERSION=latest

FROM matrixdotorg/synapse:${SYNAPSE_VERSION}

ENV SERVER_NAME=localhost

COPY conf/* /conf/
WORKDIR /data

# generate a signing key
RUN generate_signing_key -o /conf/server.signing.key
Copy link
Member

Choose a reason for hiding this comment

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

do we not still need a signing key? or is that generated somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't something I've touched (although the overall diff will make that less than obvious) — presumably it generated a signing key before. That said, this one eluded me for a bit!

It's in start.py (called by configure_workers_and_start.py):

synapse/docker/start.py

Lines 115 to 126 in 7f92ac4

# Hopefully we already have a signing key, but generate one if not.
args = [
sys.executable,
"-m",
"synapse.app.homeserver",
"--config-path",
config_path,
# tell synapse to put generated keys in /data rather than /compiled
"--keys-directory",
config_dir,
"--generate-keys",
]

Copy link
Member

Choose a reason for hiding this comment

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

I think it is something you've changed... or at least, I don't understand why you're saying it's not.

Anyway, if start.py sorts it out, I guess that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't write that file — it was already here: e4137c1 (#12881); the diff is just confusing because I deleted the monolith-only image which is what the red part of this diff (that your comment is on) corresponds to

COPY conf-workers/postgres.supervisord.conf /etc/supervisor/conf.d/postgres.conf

WORKDIR /data
# Copy the entrypoint
COPY conf-workers/start-complement-synapse-workers.sh /

# Expose nginx's listener ports
EXPOSE 8008 8448

ENTRYPOINT ["/conf/start.sh"]
ENTRYPOINT ["/start-complement-synapse-workers.sh"]

# Update the healthcheck to have a shorter check interval
HEALTHCHECK --start-period=5s --interval=1s --timeout=1s \
CMD curl -fSs http://localhost:8008/health || exit 1
CMD /bin/sh /healthcheck.sh
40 changes: 0 additions & 40 deletions docker/complement/SynapseWorkers.Dockerfile
View file
Open in desktop

This file was deleted.

129 changes: 0 additions & 129 deletions docker/complement/conf/homeserver.yaml
View file
Open in desktop

This file was deleted.

24 changes: 0 additions & 24 deletions docker/complement/conf/log_config.yaml
View file
Open in desktop

This file was deleted.

0 ...nt/conf-workers/postgres.supervisord.conf → ...complement/conf/postgres.supervisord.conf
View file
Open in desktop
File renamed without changes.
0 ...rkers/start-complement-synapse-workers.sh → .../conf/start-complement-synapse-workers.sh
View file
Open in desktop
File renamed without changes.
30 changes: 0 additions & 30 deletions docker/complement/conf/start.sh
View file
Open in desktop

This file was deleted.

0 ...mplement/conf-workers/workers-shared.yaml → docker/complement/conf/workers-shared.yaml
View file
Open in desktop
File renamed without changes.