Skip to content

Commit 2e1b04c

Browse files
MiaAltieriGu1nness
MiaAltieri
authored and
Gu1nness
committed
Updates to statefulset patching, remove needs_new_termination_period, localise use of self.first_time_with_new_termination_period, and add retry to start
1 parent b1c40ae commit 2e1b04c

File tree

2 files changed

+80
-60
lines changed

2 files changed

+80
-60
lines changed

‎src/charm.py

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,11 @@ def __init__(self, *args):
192192
)
193193

194194
# BEGIN: properties
195+
@property
196+
def needs_new_termination_period(self) -> bool:
197+
"""Returns True the termination period is incorrect."""
198+
return self.get_termination_period_for_statefulset() != ONE_YEAR
199+
195200
@property
196201
def mutator_service_name(self):
197202
"""Property to get the mutator service name for k8s."""
@@ -444,21 +449,6 @@ def db_initialised(self) -> bool:
444449
"""Check if MongoDB is initialised."""
445450
return json.loads(self.app_peer_data.get("db_initialised", "false"))
446451

447-
@property
448-
def needs_new_termination_period(self) -> bool:
449-
"""Check if the charm needs a new termination period."""
450-
return json.loads(self.app_peer_data.get("needs_new_termination_period", "true"))
451-
452-
@needs_new_termination_period.setter
453-
def needs_new_termination_period(self, value):
454-
"""Set the needs_new_termination_period flag."""
455-
if isinstance(value, bool):
456-
self.app_peer_data["needs_new_termination_period"] = json.dumps(value)
457-
else:
458-
raise ValueError(
459-
f"'needs_new_termination_period' must be a boolean value. Provided: {value} is of type {type(value)}"
460-
)
461-
462452
@property
463453
def first_time_with_new_termination_period(self) -> bool:
464454
"""Whether the unit has departed or not."""
@@ -715,14 +705,6 @@ def _on_webhook_mutator_pebble_ready(self, event) -> None:
715705
client, self.unit, self.model.name, cert, self.mutator_service_name
716706
)
717707

718-
# We must ensure that juju does not overwrite our termination period, so we should update
719-
# it as needed. However, updating the termination period can result in an onslaught of
720-
# events, including the upgrade event.
721-
# To prevent this from messing with upgrades do not update the termination period when an
722-
# upgrade is occurring.
723-
if self.get_current_termination_period() != ONE_YEAR and not self.upgrade_in_progress:
724-
self.update_termination_grace_period_to_one_year()
725-
726708
def _configure_layers(self, container: Container) -> None:
727709
"""Configure the layers of the container."""
728710
modified = False
@@ -791,7 +773,6 @@ def _on_upgrade(self, event: UpgradeCharmEvent) -> None:
791773
self.first_time_with_new_termination_period = False
792774
return
793775
if self.unit.is_leader():
794-
self.needs_new_termination_period = False
795776
self.version_checker.set_version_across_all_relations()
796777

797778
container = self.unit.get_container(Config.CONTAINER_NAME)
@@ -882,6 +863,20 @@ def _on_start(self, event: StartEvent) -> None:
882863
It is needed to install mongodb-clients inside the charm container
883864
to make this function work correctly.
884865
"""
866+
# We must ensure that juju does not overwrite our termination period, so we should update
867+
# it as needed. However, updating the termination period can result in an onslaught of
868+
# events, including the upgrade event.
869+
# To prevent this from messing with upgrades do not update the termination period when an
870+
# upgrade is occurring.
871+
if (
872+
self.unit.is_leader()
873+
and self.needs_new_termination_period
874+
and not self.upgrade_in_progress
875+
):
876+
self.update_termination_grace_period_to_one_year()
877+
event.defer()
878+
return
879+
885880
if not self.__can_charm_start():
886881
event.defer()
887882
return
@@ -985,7 +980,7 @@ def _reconcile_mongo_hosts_and_users(self, event: RelationEvent) -> None:
985980
logger.info("Deferring reconfigure: error=%r", e)
986981
event.defer()
987982

988-
def get_current_termination_period(self) -> int:
983+
def get_termination_period_for_statefulset(self) -> int:
989984
"""Returns the current termination period for the stateful set of this juju application."""
990985
client = Client()
991986
statefulset = client.get(StatefulSet, name=self.app.name, namespace=self.model.name)
@@ -1002,23 +997,26 @@ def get_termination_period_for_pod(self) -> int:
1002997
def update_termination_grace_period_to_one_year(self) -> None:
1003998
"""Patch the termination grace period for the stateful set of this juju application."""
1004999
client = Client()
1005-
patch_data = {
1006-
"spec": {
1007-
"template": {
1008-
"spec": {"terminationGracePeriodSeconds": ONE_YEAR},
1009-
"metadata": {"annotations": {"force-update": str(int(time.time()))}},
1000+
1001+
# Attempts to rewrite the terminationGracePeriodSeconds can fail if the fastapi service is
1002+
# not yet running, so we retry to give it some time settle.
1003+
for attempt in Retrying(stop=stop_after_attempt(30), wait=wait_fixed(1), reraise=True):
1004+
with attempt:
1005+
patch_data = {
1006+
"spec": {
1007+
"template": {
1008+
"spec": {"terminationGracePeriodSeconds": ONE_YEAR},
1009+
"metadata": {"annotations": {"force-update": str(int(time.time()))}},
1010+
}
1011+
}
10101012
}
1011-
}
1012-
}
1013-
client.patch(
1014-
StatefulSet,
1015-
name=self.app.name,
1016-
namespace=self.model.name,
1017-
obj=patch_data,
1018-
patch_type=PatchType.MERGE,
1019-
)
1020-
# Works because we're leader.
1021-
self.needs_new_termination_period = False
1013+
client.patch(
1014+
StatefulSet,
1015+
name=self.app.name,
1016+
namespace=self.model.name,
1017+
obj=patch_data,
1018+
patch_type=PatchType.MERGE,
1019+
)
10221020

10231021
def __handle_partition_on_stop(self) -> None:
10241022
"""Raise partition to prevent other units from restarting if an upgrade is in progress.
@@ -1182,17 +1180,14 @@ def _handle_termination(self):
11821180
events, including the upgrade event. To prevent this from messing with upgrades do not
11831181
update the termination period when an upgrade is occurring.
11841182
"""
1185-
if self.get_termination_period_for_pod() == ONE_YEAR:
1186-
self.first_time_with_new_termination_period = False
11871183
if not self.unit.is_leader():
11881184
return
11891185
try:
1190-
if self.get_current_termination_period() != ONE_YEAR and not self.upgrade_in_progress:
1186+
if self.needs_new_termination_period and not self.upgrade_in_progress:
11911187
self.update_termination_grace_period_to_one_year()
11921188
except ApiError:
11931189
logger.info("Failed to update termination period.")
11941190
return
1195-
self.needs_new_termination_period = False
11961191

11971192
# BEGIN: actions
11981193
def _on_get_password(self, event: ActionEvent) -> None:

‎tests/unit/test_charm.py

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import logging
55
import unittest
66
from unittest import mock
7-
from unittest.mock import MagicMock, patch
7+
from unittest.mock import MagicMock, PropertyMock, patch
88

99
import pytest
1010
from charms.mongodb.v1.helpers import CONF_DIR, DATA_DIR, KEY_FILE
@@ -133,8 +133,12 @@ def test_mongod_pebble_ready(self, connect_exporter, fix_data_dir, defer, pull_l
133133
# Ensure that _connect_mongodb_exporter was called
134134
connect_exporter.assert_called_once()
135135

136+
@patch(
137+
"charm.MongoDBCharm.needs_new_termination_period",
138+
new_callable=PropertyMock(return_value=False),
139+
)
136140
@patch("charm.gen_certificate", return_value=(b"", b""))
137-
@patch("charm.MongoDBCharm.get_current_termination_period")
141+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
138142
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
139143
@patch("ops.framework.EventBase.defer")
140144
@patch("charm.MongoDBCharm._push_keyfile_to_workload")
@@ -162,7 +166,7 @@ def test_pebble_ready_cannot_retrieve_container(
162166

163167
@patch("charm.gen_certificate", return_value=(b"", b""))
164168
@patch("charm.gen_certificate", return_value=(b"", b""))
165-
@patch("charm.MongoDBCharm.get_current_termination_period")
169+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
166170
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
167171
@patch("ops.framework.EventBase.defer")
168172
@patch("charm.MongoDBCharm._push_keyfile_to_workload")
@@ -228,8 +232,12 @@ def test_pebble_ready_no_storage_yet(self, defer):
228232
mock_container.replan.assert_not_called()
229233
defer.assert_called()
230234

235+
@patch(
236+
"charm.MongoDBCharm.needs_new_termination_period",
237+
new_callable=PropertyMock(return_value=False),
238+
)
231239
@patch("charm.gen_certificate", return_value=(b"", b""))
232-
@patch("charm.MongoDBCharm.get_current_termination_period")
240+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
233241
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
234242
@patch("ops.framework.EventBase.defer")
235243
@patch("charm.MongoDBProvider")
@@ -261,7 +269,7 @@ def test_start_cannot_retrieve_container(
261269
defer.assert_not_called()
262270

263271
@patch("charm.gen_certificate", return_value=(b"", b""))
264-
@patch("charm.MongoDBCharm.get_current_termination_period")
272+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
265273
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
266274
@patch("ops.framework.EventBase.defer")
267275
@patch("charm.MongoDBProvider")
@@ -291,7 +299,7 @@ def test_start_container_cannot_connect(self, connection, init_user, provider, d
291299
defer.assert_called()
292300

293301
@patch("charm.gen_certificate", return_value=(b"", b""))
294-
@patch("charm.MongoDBCharm.get_current_termination_period")
302+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
295303
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
296304
@patch("ops.framework.EventBase.defer")
297305
@patch("charm.MongoDBProvider")
@@ -321,8 +329,12 @@ def test_start_container_does_not_exist(self, connection, init_user, provider, d
321329
self.assertEqual("db_initialised" in self.harness.charm.app_peer_data, False)
322330
defer.assert_called()
323331

332+
@patch(
333+
"charm.MongoDBCharm.needs_new_termination_period",
334+
new_callable=PropertyMock(return_value=False),
335+
)
324336
@patch("charm.gen_certificate", return_value=(b"", b""))
325-
@patch("charm.MongoDBCharm.get_current_termination_period")
337+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
326338
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
327339
@patch("charm.MongoDBCharm._configure_container", return_value=None)
328340
@patch("ops.framework.EventBase.defer")
@@ -354,7 +366,11 @@ def test_start_container_exists_fails(self, connection, init_user, provider, def
354366
self.assertEqual("db_initialised" in self.harness.charm.app_peer_data, False)
355367
defer.assert_not_called()
356368

357-
@patch("charm.MongoDBCharm.get_current_termination_period")
369+
@patch(
370+
"charm.MongoDBCharm.needs_new_termination_period",
371+
new_callable=PropertyMock(return_value=False),
372+
)
373+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
358374
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
359375
@patch("charm.MongoDBCharm._configure_container", return_value=None)
360376
@patch("charm.gen_certificate", return_value=(b"", b""))
@@ -388,7 +404,7 @@ def test_start_already_initialised(self, connection, init_user, provider, defer,
388404
defer.assert_not_called()
389405

390406
@patch("charm.gen_certificate", return_value=(b"", b""))
391-
@patch("charm.MongoDBCharm.get_current_termination_period")
407+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
392408
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
393409
@patch("ops.framework.EventBase.defer")
394410
@patch("charm.MongoDBProvider")
@@ -421,7 +437,7 @@ def test_start_mongod_not_ready(self, connection, init_user, provider, defer, *u
421437
self.assertEqual("db_initialised" in self.harness.charm.app_peer_data, False)
422438
defer.assert_called()
423439

424-
@patch("charm.MongoDBCharm.get_current_termination_period")
440+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
425441
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
426442
@patch("charm.gen_certificate", return_value=(b"", b""))
427443
@patch("ops.framework.EventBase.defer")
@@ -454,7 +470,7 @@ def test_start_mongod_error_initalising_replica_set(
454470
defer.assert_called()
455471

456472
@patch("charm.gen_certificate", return_value=(b"", b""))
457-
@patch("charm.MongoDBCharm.get_current_termination_period")
473+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
458474
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
459475
@patch("ops.framework.EventBase.defer")
460476
@patch("charm.MongoDBProvider")
@@ -486,7 +502,7 @@ def test_error_initialising_users(self, connection, init_user, provider, defer,
486502
# verify app data
487503
self.assertEqual("db_initialised" in self.harness.charm.app_peer_data, False)
488504

489-
@patch("charm.MongoDBCharm.get_current_termination_period")
505+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
490506
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
491507
@patch("charm.MongoDBCharm._init_operator_user")
492508
@patch("charm.gen_certificate", return_value=(b"", b""))
@@ -701,7 +717,11 @@ def test_reconfigure_add_member_failure(self, connection, defer, *unused):
701717
connection.return_value.__enter__.return_value.add_replset_member.assert_called()
702718
defer.assert_called()
703719

704-
@patch("charm.MongoDBCharm.get_current_termination_period")
720+
@patch(
721+
"charm.MongoDBCharm.needs_new_termination_period",
722+
new_callable=PropertyMock(return_value=False),
723+
)
724+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
705725
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
706726
@patch("charm.MongoDBCharm._configure_container", return_value=None)
707727
@patch("charm.gen_certificate", return_value=(b"", b""))
@@ -1076,8 +1096,13 @@ def test__connect_mongodb_exporter_success(
10761096
expected_uri = uri_template.format(password="mongo123")
10771097
self.assertEqual(expected_uri, new_uri)
10781098

1079-
@patch("charm.MongoDBCharm.get_current_termination_period")
1099+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
10801100
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
1101+
@patch(
1102+
"charm.MongoDBCharm.needs_new_termination_period",
1103+
new_callable=PropertyMock(return_value=False),
1104+
)
1105+
@patch("charm.MongoDBCharm.needs_new_termination_period", return_value=False)
10811106
@patch("tenacity.nap.time.sleep", MagicMock())
10821107
@patch("charm.USER_CREATING_MAX_ATTEMPTS", 1)
10831108
@patch("charm.USER_CREATION_COOLDOWN", 1)
@@ -1104,7 +1129,7 @@ def test_backup_user_created(self, *unused):
11041129
self.assertIsNotNone(password) # verify the password is set
11051130

11061131
@patch("charm.gen_certificate", return_value=(b"", b""))
1107-
@patch("charm.MongoDBCharm.get_current_termination_period")
1132+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
11081133
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
11091134
@patch("charm.MongoDBConnection")
11101135
def test_set_password_provided(self, *unused):

0 commit comments

Comments
 (0)