Skip to content

Commit

Permalink
Update outdated charm libs + fix failing unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
shayancanonical committed Jun 4, 2024
1 parent 40c40d7 commit 7530e9d
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 18 deletions.
6 changes: 5 additions & 1 deletion lib/charms/data_platform_libs/v0/data_interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ def _on_topic_requested(self, event: TopicRequestedEvent):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 36
LIBPATCH = 37

PYDEPS = ["ops>=2.0.0"]

Expand Down Expand Up @@ -658,6 +658,10 @@ def set_content(self, content: Dict[str, str]) -> None:
if not self.meta:
return

# DPE-4182: do not create new revision if the content stay the same
if content == self.get_content():
return

if content:
self._move_to_new_label_if_needed()
self.meta.set_content(content)
Expand Down
7 changes: 5 additions & 2 deletions lib/charms/operator_libs_linux/v2/snap.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 6
LIBPATCH = 7


# Regex to locate 7-bit C1 ANSI sequences
Expand Down Expand Up @@ -319,7 +319,10 @@ def get(self, key: Optional[str], *, typed: bool = False) -> Any:
Default is to return a string.
"""
if typed:
config = json.loads(self._snap("get", ["-d", key]))
args = ["-d"]
if key:
args.append(key)
config = json.loads(self._snap("get", args))
if key:
return config.get(key)
return config
Expand Down
18 changes: 14 additions & 4 deletions lib/charms/rolling_ops/v0/rollingops.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def _restart(self, event):
To kick off the rolling restart, emit this library's AcquireLock event. The simplest way
to do so would be with an action, though it might make sense to acquire the lock in
response to another event.
response to another event.
```python
def _on_trigger_restart(self, event):
Expand Down Expand Up @@ -88,7 +88,7 @@ def _on_trigger_restart(self, event):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 5
LIBPATCH = 7


class LockNoRelationError(Exception):
Expand Down Expand Up @@ -182,6 +182,7 @@ def _state(self) -> LockState:
# Active acquire request.
return LockState.ACQUIRE

logger.debug("Lock state: %s %s", unit_state, app_state)
return app_state # Granted or unset/released

@_state.setter
Expand All @@ -202,21 +203,27 @@ def _state(self, state: LockState):
if state is LockState.IDLE:
self.relation.data[self.app].update({str(self.unit): state.value})

logger.debug("state: %s", state.value)

def acquire(self):
"""Request that a lock be acquired."""
self._state = LockState.ACQUIRE
logger.debug("Lock acquired.")

def release(self):
"""Request that a lock be released."""
self._state = LockState.RELEASE
logger.debug("Lock released.")

def clear(self):
"""Unset a lock."""
self._state = LockState.IDLE
logger.debug("Lock cleared.")

def grant(self):
"""Grant a lock to a unit."""
self._state = LockState.GRANTED
logger.debug("Lock granted.")

def is_held(self):
"""This unit holds the lock."""
Expand Down Expand Up @@ -266,9 +273,11 @@ def __init__(self, handle, callback_override: Optional[str] = None):
self.callback_override = callback_override or ""

def snapshot(self):
"""Snapshot of lock event."""
return {"callback_override": self.callback_override}

def restore(self, snapshot):
"""Restores lock event."""
self.callback_override = snapshot["callback_override"]


Expand All @@ -288,7 +297,7 @@ def __init__(self, charm: CharmBase, relation: AnyStr, callback: Callable):
charm: the charm we are attaching this to.
relation: an identifier, by convention based on the name of the relation in the
metadata.yaml, which identifies this instance of RollingOperatorsFactory,
distinct from other instances that may be hanlding other events.
distinct from other instances that may be handling other events.
callback: a closure to run when we have a lock. (It must take a CharmBase object and
EventBase object as args.)
"""
Expand All @@ -309,6 +318,7 @@ def __init__(self, charm: CharmBase, relation: AnyStr, callback: Callable):
self.framework.observe(charm.on[self.name].acquire_lock, self._on_acquire_lock)
self.framework.observe(charm.on[self.name].run_with_lock, self._on_run_with_lock)
self.framework.observe(charm.on[self.name].process_locks, self._on_process_locks)
self.framework.observe(charm.on.leader_elected, self._on_process_locks)

def _callback(self: CharmBase, event: EventBase) -> None:
"""Placeholder for the function that actually runs our event.
Expand Down Expand Up @@ -381,7 +391,7 @@ def _on_acquire_lock(self: CharmBase, event: ActionEvent):
"""Request a lock."""
try:
Lock(self).acquire() # Updates relation data
# emit relation changed event in the edge case where aquire does not
# emit relation changed event in the edge case where acquire does not
relation = self.model.get_relation(self.name)

# persist callback override for eventual run
Expand Down
39 changes: 31 additions & 8 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,16 @@ def test_primary_endpoint_no_peers(harness):

@patch_network_get(private_address="1.1.1.1")
def test_on_leader_elected(harness):
with patch(
with (
patch(
"charm.PostgresqlOperatorCharm._update_relation_endpoints", new_callable=PropertyMock
) as _update_relation_endpoints, patch(
"charm.PostgresqlOperatorCharm.primary_endpoint",
new_callable=PropertyMock,
) as _primary_endpoint, patch("charm.PostgresqlOperatorCharm.update_config") as _update_config:
) as _update_relation_endpoints,
patch(
"charm.PostgresqlOperatorCharm.primary_endpoint",
new_callable=PropertyMock,
) as _primary_endpoint, patch("charm.PostgresqlOperatorCharm.update_config") as _update_config,
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
# Assert that there is no password in the peer relation.
assert harness.charm._peers.data[harness.charm.app].get("operator-password", None) is None

Expand Down Expand Up @@ -570,6 +574,7 @@ def test_on_start(harness):
"charm.PostgresqlOperatorCharm._is_storage_attached",
side_effect=[False, True, True, True, True],
) as _is_storage_attached,
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
_get_postgresql_version.return_value = "14.0"

Expand Down Expand Up @@ -690,6 +695,7 @@ def test_on_start_no_patroni_member(harness):
patch(
"charm.PostgresqlOperatorCharm._is_storage_attached", return_value=True
) as _is_storage_attached,
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
# Mock the passwords.
patroni.return_value.member_started = False
Expand Down Expand Up @@ -731,7 +737,10 @@ def test_on_start_after_blocked_state(harness):

@patch_network_get(private_address="1.1.1.1")
def test_on_get_password(harness):
with patch("charm.PostgresqlOperatorCharm.update_config"):
with (
patch("charm.PostgresqlOperatorCharm.update_config"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
rel_id = harness.model.get_relation(PEER).id
# Create a mock event and set passwords in peer relation data.
harness.set_leader(True)
Expand Down Expand Up @@ -772,6 +781,7 @@ def test_on_set_password(harness):
patch("charm.PostgresqlOperatorCharm.postgresql") as _postgresql,
patch("charm.Patroni.are_all_members_ready") as _are_all_members_ready,
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
# Create a mock event.
mock_event = MagicMock(params={})
Expand Down Expand Up @@ -1738,7 +1748,10 @@ def test_get_secret_from_databag(harness):
This must be backwards-compatible so it runs on both juju2 and juju3.
"""
with patch("charm.PostgresqlOperatorCharm._on_leader_elected"):
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
rel_id = harness.model.get_relation(PEER).id
# App level changes require leader privileges
harness.set_leader()
Expand All @@ -1763,6 +1776,7 @@ def test_get_secret_from_databag(harness):
def test_on_get_password_secrets(harness):
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
# Create a mock event and set passwords in peer relation data.
harness.set_leader()
Expand Down Expand Up @@ -1794,6 +1808,7 @@ def test_on_get_password_secrets(harness):
def test_get_secret_secrets(harness, scope, field):
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
harness.set_leader()

Expand All @@ -1808,7 +1823,10 @@ def test_set_secret_in_databag(harness, only_without_juju_secrets):
This is juju2 specific. In juju3, set_secret writes to juju secrets.
"""
with patch("charm.PostgresqlOperatorCharm._on_leader_elected"):
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
rel_id = harness.model.get_relation(PEER).id
harness.set_leader()

Expand Down Expand Up @@ -1841,6 +1859,7 @@ def test_set_secret_in_databag(harness, only_without_juju_secrets):
def test_set_reset_new_secret(harness, scope, is_leader):
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
"""NOTE: currently ops.testing seems to allow for non-leader to set secrets too!"""
# App has to be leader, unit can be either
Expand All @@ -1863,6 +1882,7 @@ def test_set_reset_new_secret(harness, scope, is_leader):
def test_invalid_secret(harness, scope, is_leader):
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
# App has to be leader, unit can be either
harness.set_leader(is_leader)
Expand All @@ -1878,6 +1898,7 @@ def test_invalid_secret(harness, scope, is_leader):
def test_delete_password(harness, _has_secrets, caplog):
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
"""NOTE: currently ops.testing seems to allow for non-leader to remove secrets too!"""
harness.set_leader(True)
Expand Down Expand Up @@ -1929,6 +1950,7 @@ def test_migration_from_databag(harness, only_with_juju_secrets, scope, is_leade
"""
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
rel_id = harness.model.get_relation(PEER).id
# App has to be leader, unit can be either
Expand Down Expand Up @@ -1957,6 +1979,7 @@ def test_migration_from_single_secret(harness, only_with_juju_secrets, scope, is
"""
with (
patch("charm.PostgresqlOperatorCharm._on_leader_elected"),
patch("charms.rolling_ops.v0.rollingops.RollingOpsManager._on_process_locks"),
):
rel_id = harness.model.get_relation(PEER).id

Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ def test_on_upgrade_granted(harness):
with (
patch("charm.Patroni.get_postgresql_version"),
patch(
"charms.data_platform_libs.v0.upgrade.DataUpgrade.on_upgrade_changed"
"charm.PostgreSQLUpgrade.on_upgrade_changed"
) as _on_upgrade_changed,
patch(
"charms.data_platform_libs.v0.upgrade.DataUpgrade.set_unit_failed"
"charm.PostgreSQLUpgrade.set_unit_failed"
) as _set_unit_failed,
patch(
"charms.data_platform_libs.v0.upgrade.DataUpgrade.set_unit_completed"
"charm.PostgreSQLUpgrade.set_unit_completed"
) as _set_unit_completed,
patch(
"charm.Patroni.is_replication_healthy", new_callable=PropertyMock
Expand Down

0 comments on commit 7530e9d

Please sign in to comment.