Skip to content

ISD-2819 restart synapse and MAS if MAS config has changed #779

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 33 additions & 2 deletions src/pebble.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,28 @@ def _get_synapse_config(container: ops.model.Container) -> dict:
raise PebbleServiceError(str(exc)) from exc


def _get_mas_config(container: ops.model.Container) -> dict:
"""Get the current MAS configuration.

Args:
container: Synapse container.

Returns:
dict: MAS configuration.

Raises:
PebbleServiceError: if something goes wrong while interacting with Pebble.
"""
try:
config = container.pull(MAS_CONFIGURATION_PATH).read()
return yaml.safe_load(config)
except ops.pebble.PathError as exc:
# If the MAS config has not been created, we return an empty dict to trigger a replan
Copy link
Collaborator

Choose a reason for hiding this comment

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

When could this happen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The 2 most likely cases are:
When the charm first starts, or when the config is manually deleted for whatever reason

if exc.kind == "not-found":
return {}
raise PebbleServiceError(str(exc)) from exc


def _push_synapse_config(
container: ops.model.Container,
current_synapse_config: dict,
Expand Down Expand Up @@ -317,7 +339,16 @@ def reconcile( # noqa: C901
PebbleServiceError: if something goes wrong while interacting with Pebble.
"""
try:
restart_mas(container, rendered_mas_configuration, charm_state)

existing_mas_config = _get_mas_config(container=container)
mas_config_has_changed = DeepDiff(
existing_mas_config,
yaml.safe_load(rendered_mas_configuration),
ignore_order=True,
ignore_string_case=True,
)
if mas_config_has_changed:
restart_mas(container, rendered_mas_configuration, charm_state)

if _environment_has_changed(container=container, charm_state=charm_state, is_main=is_main):
# Configurations set via environment variables:
Expand Down Expand Up @@ -402,7 +433,7 @@ def reconcile( # noqa: C901
ignore_string_case=True,
)

if config_has_changed:
if config_has_changed or mas_config_has_changed:
logging.info("Configuration has changed, Synapse will be restarted.")
logging.debug("The change is: %s", config_has_changed)
# Push worker configuration
Expand Down
9 changes: 8 additions & 1 deletion tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from ops.testing import Harness

import synapse
from auth.mas import MAS_EXECUTABLE_PATH
from auth.mas import MAS_CONFIGURATION_PATH, MAS_EXECUTABLE_PATH
from charm import SynapseCharm
from s3_parameters import S3Parameters

Expand Down Expand Up @@ -139,6 +139,13 @@ def harness_fixture(request, monkeypatch) -> typing.Generator[Harness, None, Non
harness.set_can_connect(synapse.SYNAPSE_CONTAINER_NAME, True)
synapse_container.make_dir("/data", make_parents=True)
synapse_container.push(f"/data/{TEST_SERVER_NAME}.signing.key", "123")
synapse_container.make_dir("/mas", make_parents=True)
synapse_container.push(
MAS_CONFIGURATION_PATH,
yaml.safe_dump(
{"http": {"listeners": [{"name": "web", "binds": [{"address": "[::]:8081"}]}]}}
),
)
# unused-variable disabled to pass constants values to inner function
command_path = synapse.SYNAPSE_COMMAND_PATH
command_migrate_config = synapse.COMMAND_MIGRATE_CONFIG
Expand Down
Loading