Skip to content

Commit bd3f9f7

Browse files
MiaAltieriMiaAltieri
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 bd3f9f7

File tree

2 files changed

+55
-66
lines changed

2 files changed

+55
-66
lines changed

‎src/charm.py

Lines changed: 41 additions & 52 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
@@ -899,9 +894,6 @@ def _on_start(self, event: StartEvent) -> None:
899894
self.status.set_and_share_status(ActiveStatus())
900895
self.upgrade._reconcile_upgrade(event)
901896

902-
if self.get_termination_period_for_pod() == ONE_YEAR:
903-
self.first_time_with_new_termination_period = False
904-
905897
if not self.unit.is_leader():
906898
return
907899

@@ -985,7 +977,7 @@ def _reconcile_mongo_hosts_and_users(self, event: RelationEvent) -> None:
985977
logger.info("Deferring reconfigure: error=%r", e)
986978
event.defer()
987979

988-
def get_current_termination_period(self) -> int:
980+
def get_termination_period_for_statefulset(self) -> int:
989981
"""Returns the current termination period for the stateful set of this juju application."""
990982
client = Client()
991983
statefulset = client.get(StatefulSet, name=self.app.name, namespace=self.model.name)
@@ -1002,23 +994,26 @@ def get_termination_period_for_pod(self) -> int:
1002994
def update_termination_grace_period_to_one_year(self) -> None:
1003995
"""Patch the termination grace period for the stateful set of this juju application."""
1004996
client = Client()
1005-
patch_data = {
1006-
"spec": {
1007-
"template": {
1008-
"spec": {"terminationGracePeriodSeconds": ONE_YEAR},
1009-
"metadata": {"annotations": {"force-update": str(int(time.time()))}},
997+
998+
# Attempts to rewrite the terminationGracePeriodSeconds can fail if the fastapi service is
999+
# not yet running, so we retry to give it some time settle.
1000+
for attempt in Retrying(stop=stop_after_attempt(30), wait=wait_fixed(1), reraise=True):
1001+
with attempt:
1002+
patch_data = {
1003+
"spec": {
1004+
"template": {
1005+
"spec": {"terminationGracePeriodSeconds": ONE_YEAR},
1006+
"metadata": {"annotations": {"force-update": str(int(time.time()))}},
1007+
}
1008+
}
10101009
}
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
1010+
client.patch(
1011+
StatefulSet,
1012+
name=self.app.name,
1013+
namespace=self.model.name,
1014+
obj=patch_data,
1015+
patch_type=PatchType.MERGE,
1016+
)
10221017

10231018
def __handle_partition_on_stop(self) -> None:
10241019
"""Raise partition to prevent other units from restarting if an upgrade is in progress.
@@ -1067,10 +1062,7 @@ def _delete_service(self):
10671062
logger.error(str(err))
10681063

10691064
def _on_stop(self, event) -> None:
1070-
if (
1071-
not self.needs_new_termination_period
1072-
and not self.first_time_with_new_termination_period
1073-
):
1065+
if not self.needs_new_termination_period:
10741066
self.__handle_partition_on_stop()
10751067
if self._is_removing_last_replica:
10761068
self._delete_service()
@@ -1182,17 +1174,14 @@ def _handle_termination(self):
11821174
events, including the upgrade event. To prevent this from messing with upgrades do not
11831175
update the termination period when an upgrade is occurring.
11841176
"""
1185-
if self.get_termination_period_for_pod() == ONE_YEAR:
1186-
self.first_time_with_new_termination_period = False
11871177
if not self.unit.is_leader():
11881178
return
11891179
try:
1190-
if self.get_current_termination_period() != ONE_YEAR and not self.upgrade_in_progress:
1180+
if self.needs_new_termination_period and not self.upgrade_in_progress:
11911181
self.update_termination_grace_period_to_one_year()
11921182
except ApiError:
11931183
logger.info("Failed to update termination period.")
11941184
return
1195-
self.needs_new_termination_period = False
11961185

11971186
# BEGIN: actions
11981187
def _on_get_password(self, event: ActionEvent) -> None:

‎tests/unit/test_charm.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ def test_mongod_pebble_ready(self, connect_exporter, fix_data_dir, defer, pull_l
134134
connect_exporter.assert_called_once()
135135

136136
@patch("charm.gen_certificate", return_value=(b"", b""))
137-
@patch("charm.MongoDBCharm.get_current_termination_period")
137+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
138138
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
139139
@patch("ops.framework.EventBase.defer")
140140
@patch("charm.MongoDBCharm._push_keyfile_to_workload")
@@ -162,7 +162,7 @@ def test_pebble_ready_cannot_retrieve_container(
162162

163163
@patch("charm.gen_certificate", return_value=(b"", b""))
164164
@patch("charm.gen_certificate", return_value=(b"", b""))
165-
@patch("charm.MongoDBCharm.get_current_termination_period")
165+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
166166
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
167167
@patch("ops.framework.EventBase.defer")
168168
@patch("charm.MongoDBCharm._push_keyfile_to_workload")
@@ -229,7 +229,7 @@ def test_pebble_ready_no_storage_yet(self, defer):
229229
defer.assert_called()
230230

231231
@patch("charm.gen_certificate", return_value=(b"", b""))
232-
@patch("charm.MongoDBCharm.get_current_termination_period")
232+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
233233
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
234234
@patch("ops.framework.EventBase.defer")
235235
@patch("charm.MongoDBProvider")
@@ -261,7 +261,7 @@ def test_start_cannot_retrieve_container(
261261
defer.assert_not_called()
262262

263263
@patch("charm.gen_certificate", return_value=(b"", b""))
264-
@patch("charm.MongoDBCharm.get_current_termination_period")
264+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
265265
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
266266
@patch("ops.framework.EventBase.defer")
267267
@patch("charm.MongoDBProvider")
@@ -291,7 +291,7 @@ def test_start_container_cannot_connect(self, connection, init_user, provider, d
291291
defer.assert_called()
292292

293293
@patch("charm.gen_certificate", return_value=(b"", b""))
294-
@patch("charm.MongoDBCharm.get_current_termination_period")
294+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
295295
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
296296
@patch("ops.framework.EventBase.defer")
297297
@patch("charm.MongoDBProvider")
@@ -322,7 +322,7 @@ def test_start_container_does_not_exist(self, connection, init_user, provider, d
322322
defer.assert_called()
323323

324324
@patch("charm.gen_certificate", return_value=(b"", b""))
325-
@patch("charm.MongoDBCharm.get_current_termination_period")
325+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
326326
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
327327
@patch("charm.MongoDBCharm._configure_container", return_value=None)
328328
@patch("ops.framework.EventBase.defer")
@@ -354,7 +354,7 @@ def test_start_container_exists_fails(self, connection, init_user, provider, def
354354
self.assertEqual("db_initialised" in self.harness.charm.app_peer_data, False)
355355
defer.assert_not_called()
356356

357-
@patch("charm.MongoDBCharm.get_current_termination_period")
357+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
358358
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
359359
@patch("charm.MongoDBCharm._configure_container", return_value=None)
360360
@patch("charm.gen_certificate", return_value=(b"", b""))
@@ -388,7 +388,7 @@ def test_start_already_initialised(self, connection, init_user, provider, defer,
388388
defer.assert_not_called()
389389

390390
@patch("charm.gen_certificate", return_value=(b"", b""))
391-
@patch("charm.MongoDBCharm.get_current_termination_period")
391+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
392392
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
393393
@patch("ops.framework.EventBase.defer")
394394
@patch("charm.MongoDBProvider")
@@ -421,7 +421,7 @@ def test_start_mongod_not_ready(self, connection, init_user, provider, defer, *u
421421
self.assertEqual("db_initialised" in self.harness.charm.app_peer_data, False)
422422
defer.assert_called()
423423

424-
@patch("charm.MongoDBCharm.get_current_termination_period")
424+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
425425
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
426426
@patch("charm.gen_certificate", return_value=(b"", b""))
427427
@patch("ops.framework.EventBase.defer")
@@ -454,7 +454,7 @@ def test_start_mongod_error_initalising_replica_set(
454454
defer.assert_called()
455455

456456
@patch("charm.gen_certificate", return_value=(b"", b""))
457-
@patch("charm.MongoDBCharm.get_current_termination_period")
457+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
458458
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
459459
@patch("ops.framework.EventBase.defer")
460460
@patch("charm.MongoDBProvider")
@@ -486,7 +486,7 @@ def test_error_initialising_users(self, connection, init_user, provider, defer,
486486
# verify app data
487487
self.assertEqual("db_initialised" in self.harness.charm.app_peer_data, False)
488488

489-
@patch("charm.MongoDBCharm.get_current_termination_period")
489+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
490490
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
491491
@patch("charm.MongoDBCharm._init_operator_user")
492492
@patch("charm.gen_certificate", return_value=(b"", b""))
@@ -701,7 +701,7 @@ def test_reconfigure_add_member_failure(self, connection, defer, *unused):
701701
connection.return_value.__enter__.return_value.add_replset_member.assert_called()
702702
defer.assert_called()
703703

704-
@patch("charm.MongoDBCharm.get_current_termination_period")
704+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
705705
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
706706
@patch("charm.MongoDBCharm._configure_container", return_value=None)
707707
@patch("charm.gen_certificate", return_value=(b"", b""))
@@ -1076,7 +1076,7 @@ def test__connect_mongodb_exporter_success(
10761076
expected_uri = uri_template.format(password="mongo123")
10771077
self.assertEqual(expected_uri, new_uri)
10781078

1079-
@patch("charm.MongoDBCharm.get_current_termination_period")
1079+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
10801080
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
10811081
@patch("tenacity.nap.time.sleep", MagicMock())
10821082
@patch("charm.USER_CREATING_MAX_ATTEMPTS", 1)
@@ -1104,7 +1104,7 @@ def test_backup_user_created(self, *unused):
11041104
self.assertIsNotNone(password) # verify the password is set
11051105

11061106
@patch("charm.gen_certificate", return_value=(b"", b""))
1107-
@patch("charm.MongoDBCharm.get_current_termination_period")
1107+
@patch("charm.MongoDBCharm.get_termination_period_for_statefulset")
11081108
@patch("charm.MongoDBCharm.update_termination_grace_period_to_one_year")
11091109
@patch("charm.MongoDBConnection")
11101110
def test_set_password_provided(self, *unused):

0 commit comments

Comments
 (0)