Fix a bug introduced in Synapse v1.84.0 where workers do not start up when no instance_map was provided. (#15672)

* Fix #15669: always populate instance map even if it was empty

* Fix some tests

* Fix more tests

* Newsfile

Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>

* CI fix: don't forget to update apt repository sources before installing olddeps deps

* Add test testing the backwards compatibility

---------

Signed-off-by: Olivier Wilkinson (reivilibre) <oliverw@matrix.org>
This commit is contained in:
reivilibre 2023-05-26 14:28:55 +00:00 committed by GitHub
parent 2d8a2ca374
commit c775d80b73
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 47 additions and 5 deletions

View file

@ -314,6 +314,7 @@ jobs:
# There aren't wheels for some of the older deps, so we need to install
# their build dependencies
- run: |
sudo apt-get -qq update
sudo apt-get -qq install build-essential libffi-dev python-dev \
libxml2-dev libxslt-dev xmlsec1 zlib1g-dev libjpeg-dev libwebp-dev

1
changelog.d/15672.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a bug introduced in Synapse v1.84.0 where workers do not start up when no `instance_map` was provided.

View file

@ -222,7 +222,7 @@ class WorkerConfig(Config):
# itself doesn't need this data as it would never have to talk to itself.
instance_map: Dict[str, Any] = config.get("instance_map", {})
if instance_map and self.instance_name is not MAIN_PROCESS_INSTANCE_NAME:
if self.instance_name is not MAIN_PROCESS_INSTANCE_NAME:
# The host used to connect to the main synapse
main_host = config.get("worker_replication_host", None)

View file

@ -25,6 +25,8 @@ class HomeserverAppStartTestCase(ConfigFileTestCase):
# Add a blank line as otherwise the next addition ends up on a line with a comment
self.add_lines_to_config([" "])
self.add_lines_to_config(["worker_app: test_worker_app"])
self.add_lines_to_config(["worker_replication_host: 127.0.0.1"])
self.add_lines_to_config(["worker_replication_http_port: 0"])
# Ensure that starting master process with worker config raises an exception
with self.assertRaises(ConfigError):

View file

@ -42,6 +42,7 @@ class FederationReaderOpenIDListenerTests(HomeserverTestCase):
# have to tell the FederationHandler not to try to access stuff that is only
# in the primary store.
conf["worker_app"] = "yes"
conf["instance_map"] = {"main": {"host": "127.0.0.1", "port": 0}}
return conf

View file

@ -17,7 +17,7 @@ from unittest.mock import Mock
from immutabledict import immutabledict
from synapse.config import ConfigError
from synapse.config.workers import WorkerConfig
from synapse.config.workers import InstanceLocationConfig, WorkerConfig
from tests.unittest import TestCase
@ -94,6 +94,7 @@ class WorkerDutyConfigTestCase(TestCase):
# so that it doesn't raise an exception here.
# (This is not read by `_should_this_worker_perform_duty`.)
"notify_appservices": False,
"instance_map": {"main": {"host": "127.0.0.1", "port": 0}},
},
)
@ -138,7 +139,9 @@ class WorkerDutyConfigTestCase(TestCase):
"""
main_process_config = self._make_worker_config(
worker_app="synapse.app.homeserver", worker_name=None
worker_app="synapse.app.homeserver",
worker_name=None,
extras={"instance_map": {"main": {"host": "127.0.0.1", "port": 0}}},
)
self.assertTrue(
@ -203,6 +206,7 @@ class WorkerDutyConfigTestCase(TestCase):
# so that it doesn't raise an exception here.
# (This is not read by `_should_this_worker_perform_duty`.)
"notify_appservices": False,
"instance_map": {"main": {"host": "127.0.0.1", "port": 0}},
},
)
@ -236,7 +240,9 @@ class WorkerDutyConfigTestCase(TestCase):
Tests new config options. This is for the master's config.
"""
main_process_config = self._make_worker_config(
worker_app="synapse.app.homeserver", worker_name=None
worker_app="synapse.app.homeserver",
worker_name=None,
extras={"instance_map": {"main": {"host": "127.0.0.1", "port": 0}}},
)
self.assertTrue(
@ -262,7 +268,9 @@ class WorkerDutyConfigTestCase(TestCase):
Tests new config options. This is for the worker's config.
"""
appservice_worker_config = self._make_worker_config(
worker_app="synapse.app.generic_worker", worker_name="worker1"
worker_app="synapse.app.generic_worker",
worker_name="worker1",
extras={"instance_map": {"main": {"host": "127.0.0.1", "port": 0}}},
)
self.assertTrue(
@ -298,6 +306,7 @@ class WorkerDutyConfigTestCase(TestCase):
extras={
"notify_appservices_from_worker": "worker2",
"update_user_directory_from_worker": "worker1",
"instance_map": {"main": {"host": "127.0.0.1", "port": 0}},
},
)
self.assertFalse(worker1_config.should_notify_appservices)
@ -309,7 +318,33 @@ class WorkerDutyConfigTestCase(TestCase):
extras={
"notify_appservices_from_worker": "worker2",
"update_user_directory_from_worker": "worker1",
"instance_map": {"main": {"host": "127.0.0.1", "port": 0}},
},
)
self.assertTrue(worker2_config.should_notify_appservices)
self.assertFalse(worker2_config.should_update_user_directory)
def test_worker_instance_map_compat(self) -> None:
"""
Test that `worker_replication_*` settings are compatibly handled by
adding them to the instance map as a `main` entry.
"""
worker1_config = self._make_worker_config(
worker_app="synapse.app.generic_worker",
worker_name="worker1",
extras={
"notify_appservices_from_worker": "worker2",
"update_user_directory_from_worker": "worker1",
"worker_replication_host": "127.0.0.42",
"worker_replication_http_port": 1979,
},
)
self.assertEqual(
worker1_config.instance_map,
{
"master": InstanceLocationConfig(
host="127.0.0.42", port=1979, tls=False
),
},
)

View file

@ -32,6 +32,7 @@ class FederationAckTestCase(HomeserverTestCase):
config["worker_app"] = "synapse.app.generic_worker"
config["worker_name"] = "federation_sender1"
config["federation_sender_instances"] = ["federation_sender1"]
config["instance_map"] = {"main": {"host": "127.0.0.1", "port": 0}}
return config
def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:

View file

@ -55,6 +55,7 @@ class WorkerSchemaTests(HomeserverTestCase):
# Mark this as a worker app.
conf["worker_app"] = "yes"
conf["instance_map"] = {"main": {"host": "127.0.0.1", "port": 0}}
return conf