Skip to content

Commit

Permalink
Feat/ Redis HA fix (#283)
Browse files Browse the repository at this point in the history
* feat: Read Redis Master Ip from app databag

* chore: Improve unit test to include testing for relation data

* chore: Fix linting for noble systems

* chore: Run

* chore: Small refactor

* chore: Add backwards compatibility to Redis relation and its tests

* chore: Refactor
  • Loading branch information
alithethird committed Sep 10, 2024
1 parent 381c979 commit 8175cc2
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 11 deletions.
1 change: 0 additions & 1 deletion src-docs/charm.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Charm for Discourse on kubernetes.
- **DEFAULT_RELATION_NAME**
- **DATABASE_NAME**
- **DISCOURSE_PATH**
- **THROTTLE_LEVELS**
- **LOG_PATHS**
- **PROMETHEUS_PORT**
- **REQUIRED_S3_SETTINGS**
Expand Down
16 changes: 11 additions & 5 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,16 +390,22 @@ def _get_redis_relation_data(self) -> typing.Tuple[str, int]:
Raises:
MissingRedisRelationDataError if the some of redis relation data is malformed/missing
"""
relation_data = self.redis.relation_data
if not relation_data:
relation = self.model.get_relation(self.redis.relation_name)
if not relation:
raise MissingRedisRelationDataError("No redis relation data")
relation_app_data = relation.data[relation.app]
relation_unit_data = self.redis.relation_data

try:
redis_hostname = str(relation_data["hostname"])
redis_port = int(relation_data["port"])
redis_hostname = str(
relation_app_data["leader-host"]
if relation_app_data.get("leader-host")
else relation_unit_data["hostname"]
)
redis_port = int(relation_unit_data["port"])
except (KeyError, ValueError) as exc:
raise MissingRedisRelationDataError(
"Wrong hostname or port in Redis relation"
"Either 'leader-host' or 'hostname' and 'ports' are mandatory"
) from exc

logger.debug(
Expand Down
8 changes: 6 additions & 2 deletions tests/unit/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,19 @@ def add_postgres_relation(harness):
)


def add_redis_relation(harness, relation_data=None):
def add_redis_relation(harness, relation_data=None, app_data=None):
"""Add redis relation and relation data to the charm.
Args:
- A harness instance
Returns: the same harness instance with an added relation
"""
redis_relation_id = harness.add_relation("redis", "redis")
redis_relation_id = harness.add_relation(
"redis",
"redis",
app_data={"leader-host": "redis-host"} if app_data is None else app_data,
)
harness.add_relation_unit(redis_relation_id, "redis/0")
# We need to bypass protected access to inject the relation data
# pylint: disable=protected-access
Expand Down
18 changes: 15 additions & 3 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -694,46 +694,58 @@ def test_is_database_relation_ready(relation_data, should_be_ready):


@pytest.mark.parametrize(
"relation_data, should_be_ready",
"relation_data, app_data, should_be_ready",
[
(
{"hostname": "redis-host", "port": "1010"},
{"leader-host": "redis-host"},
True,
),
(
{"hostname": "redis-host", "port": "1010"},
{},
True,
),
(
{"hostname": "redis-host", "port": "0"},
{"leader-host": ""},
False,
),
(
{"hostname": "", "port": "1010"},
{"leader-host": ""},
False,
),
(
{"hostname": "redis-host"},
{},
False,
),
(
{},
{},
False,
),
(
{"port": "6379"},
{},
False,
),
(
{"hostname": "redis-port"},
{},
False,
),
],
)
def test_is_redis_relation_ready(relation_data, should_be_ready):
def test_is_redis_relation_ready(relation_data, app_data, should_be_ready):
"""
arrange: given a deployed discourse charm and some relation data
act: add the redis relation to the charm
assert: the charm should wait for some correct relation data
"""
harness = helpers.start_harness(with_postgres=True, with_redis=False)
helpers.add_redis_relation(harness, relation_data)
helpers.add_redis_relation(harness, relation_data, app_data)
assert should_be_ready == harness.charm._are_relations_ready()


Expand Down
1 change: 1 addition & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ commands =

[testenv:lint]
description = Check code against coding style standards
basepython = py310
deps =
black
boto3
Expand Down

0 comments on commit 8175cc2

Please sign in to comment.