Skip to content

Commit

Permalink
pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
MiaAltieri committed Nov 1, 2023
1 parent 34b36b4 commit 679ab6e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 30 deletions.
22 changes: 11 additions & 11 deletions lib/charms/mongodb/v1/shards_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from ops.model import (
ActiveStatus,
BlockedStatus,
ErrorStatus,
MaintenanceStatus,
StatusBase,
WaitingStatus,
Expand Down Expand Up @@ -268,7 +269,7 @@ def update_mongos_hosts(self):
for relation in self.charm.model.relations[self.relation_name]:
self._update_relation_data(relation.id, {HOSTS_KEY: json.dumps(self.charm._unit_ips)})

def get_config_server_status(self):
def get_config_server_status(self) -> StatusBase:
"""Returns the current status of the config-server."""
if not self.charm.is_role(Config.Role.CONFIG_SERVER):
logger.info("skipping status check, charm is not running as a shard")
Expand Down Expand Up @@ -298,11 +299,9 @@ def get_config_server_status(self):
unreachable_shards = self.get_unreachable_shards()
if unreachable_shards:
unreachable_shards = "".join(unreachable_shards)
return BlockedStatus(f"Shards {unreachable_shards} are unreachable.")
return ErrorStatus(f"Shards {unreachable_shards} are unreachable.")

shards = self.get_related_shards()
shards = ", ".join(shards) if len(shards) < 5 else f"{len(shards)} shards"
return ActiveStatus(f"config-server connected to {shards}")
return ActiveStatus()

def _update_relation_data(self, relation_id: int, data: dict) -> None:
"""Updates a set of key-value pairs in the relation.
Expand Down Expand Up @@ -356,15 +355,15 @@ def get_related_shards(self) -> List[str]:

def get_unreachable_shards(self) -> List[str]:
"""Returns a list of unreable shard hosts."""
unreachable_hosts = []
if not self.model.relations[self.relation_name]:
logger.info("shards are not reachable, none related to config-sever")
return False
return unreachable_hosts

unreachable_hosts = []
for shard_name in self.get_related_shards():
shard_hosts = self._get_shard_hosts(shard_name)
if not shard_hosts:
return False
return unreachable_hosts

# use a URI that is not dependent on the operator password, as we are not guaranteed
# that the shard has received the password yet.
Expand All @@ -373,6 +372,8 @@ def get_unreachable_shards(self) -> List[str]:
if not mongo.is_ready:
unreachable_hosts.append(shard_name)

return unreachable_hosts

def is_mongos_running(self) -> bool:
"""Returns true if mongos service is running."""
mongos_hosts = ",".join(self.charm._unit_ips)
Expand Down Expand Up @@ -566,16 +567,15 @@ def get_shard_status(self) -> Optional[StatusBase]:
return ActiveStatus("Shard drained from cluster, ready for removal")

if not self._is_mongos_reachable():
return BlockedStatus("Config server unreachable")
return ErrorStatus("Config server unreachable")

if not self._is_added_to_cluster():
return MaintenanceStatus("Adding shard to config-server")

if not self._is_shard_aware():
return BlockedStatus("Shard is not yet shard aware")

config_server_name = self.get_related_config_server()
return ActiveStatus(f"Shard connected to config-server: {config_server_name}")
return ActiveStatus()

def drained(self, mongos_hosts: Set[str], shard_name: str) -> bool:
"""Returns whether a shard has been drained from the cluster.
Expand Down
8 changes: 1 addition & 7 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1380,13 +1380,7 @@ def get_status(self) -> StatusBase:
if pbm_status and not isinstance(pbm_status, ActiveStatus):
return pbm_status

# if all statuses are active report sharding statuses over mongodb status
if isinstance(shard_status, ActiveStatus):
return shard_status

if isinstance(config_server_status, ActiveStatus):
return config_server_status

# if all statuses are active report mongodb status over sharding status
return mongodb_status

def is_relation_feasible(self, rel_interface) -> bool:
Expand Down
12 changes: 0 additions & 12 deletions tests/integration/sharding_tests/test_sharding.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.
import asyncio
import re

import pytest
from pytest_operator.plugin import OpsTest
Expand Down Expand Up @@ -91,17 +90,6 @@ async def test_cluster_active(ops_test: OpsTest) -> None:
timeout=TIMEOUT,
)

config_server_unit = ops_test.model.applications[CONFIG_SERVER_APP_NAME].units[0]
correct_pattern = r"config-server connected to (shard-one, shard-two|shard-two, shard-one)"
assert re.match(correct_pattern, config_server_unit.workload_status_message)

for shard_app_name in SHARD_APPS:
shard_unit = ops_test.model.applications[shard_app_name].units[0]
assert (
shard_unit.workload_status_message
== f"Shard connected to config-server: {CONFIG_SERVER_APP_NAME}"
)


async def test_sharding(ops_test: OpsTest) -> None:
"""Tests writing data to mongos gets propagated to shards."""
Expand Down

0 comments on commit 679ab6e

Please sign in to comment.