Skip to content

Commit 0d3dc5b

Browse files
[DPE-1999] Fixed TLS race condition (#196)
* Fixed TLS race condition * Changed Redis channel in test * Added additional logic
1 parent 1bb0358 commit 0d3dc5b

File tree

2 files changed

+34
-10
lines changed

2 files changed

+34
-10
lines changed

lib/charms/postgresql_k8s/v0/postgresql_tls.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
)
3434
from cryptography import x509
3535
from cryptography.x509.extensions import ExtensionType
36-
from ops.charm import ActionEvent
36+
from ops.charm import ActionEvent, RelationBrokenEvent
3737
from ops.framework import Object
3838
from ops.pebble import ConnectionError, PathError, ProtocolError
3939

@@ -45,7 +45,7 @@
4545

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

5050
logger = logging.getLogger(__name__)
5151
SCOPE = "unit"
@@ -116,12 +116,14 @@ def _on_tls_relation_joined(self, _) -> None:
116116
"""Request certificate when TLS relation joined."""
117117
self._request_certificate(None)
118118

119-
def _on_tls_relation_broken(self, _) -> None:
119+
def _on_tls_relation_broken(self, event: RelationBrokenEvent) -> None:
120120
"""Disable TLS when TLS relation broken."""
121121
self.charm.set_secret(SCOPE, "ca", None)
122122
self.charm.set_secret(SCOPE, "cert", None)
123123
self.charm.set_secret(SCOPE, "chain", None)
124-
self.charm.update_config()
124+
if not self.charm.update_config():
125+
logger.debug("Cannot update config at this moment")
126+
event.defer()
125127

126128
def _on_certificate_available(self, event: CertificateAvailableEvent) -> None:
127129
"""Enable TLS when TLS certificate available."""
@@ -139,7 +141,10 @@ def _on_certificate_available(self, event: CertificateAvailableEvent) -> None:
139141
self.charm.set_secret(SCOPE, "ca", event.ca)
140142

141143
try:
142-
self.charm.push_tls_files_to_workload()
144+
if not self.charm.push_tls_files_to_workload():
145+
logger.debug("Cannot push TLS certificates at this moment")
146+
event.defer()
147+
return
143148
except (ConnectionError, PathError, ProtocolError) as e:
144149
logger.error("Cannot push TLS certificates: %r", e)
145150
event.defer()

src/charm.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,7 @@ def _peers(self) -> Relation:
10421042
"""
10431043
return self.model.get_relation(PEER)
10441044

1045-
def push_tls_files_to_workload(self, container: Container = None) -> None:
1045+
def push_tls_files_to_workload(self, container: Container = None) -> bool:
10461046
"""Uploads TLS files to the workload container."""
10471047
if container is None:
10481048
container = self.unit.get_container("postgresql")
@@ -1086,7 +1086,7 @@ def push_tls_files_to_workload(self, container: Container = None) -> None:
10861086
group=WORKLOAD_OS_GROUP,
10871087
)
10881088

1089-
self.update_config()
1089+
return self.update_config()
10901090

10911091
def _restart(self, event: RunWithLock) -> None:
10921092
"""Restart PostgreSQL."""
@@ -1109,7 +1109,20 @@ def _restart(self, event: RunWithLock) -> None:
11091109
# Start or stop the pgBackRest TLS server service when TLS certificate change.
11101110
self.backup.start_stop_pgbackrest_service()
11111111

1112-
def update_config(self) -> None:
1112+
@property
1113+
def _is_workload_running(self) -> bool:
1114+
"""Returns whether the workload is running (in an active state)."""
1115+
container = self.unit.get_container("postgresql")
1116+
if not container.can_connect():
1117+
return False
1118+
1119+
services = container.pebble.get_services(names=[self._postgresql_service])
1120+
if len(services) == 0:
1121+
return False
1122+
1123+
return services[0].current == ServiceStatus.ACTIVE
1124+
1125+
def update_config(self) -> bool:
11131126
"""Updates Patroni config file based on the existence of the TLS files."""
11141127
# Update and reload configuration based on TLS files availability.
11151128
self._patroni.render_patroni_yml_file(
@@ -1119,14 +1132,18 @@ def update_config(self) -> None:
11191132
stanza=self.app_peer_data.get("stanza"),
11201133
restore_stanza=self.app_peer_data.get("restore-stanza"),
11211134
)
1122-
if not self._patroni.member_started:
1135+
if not self._is_workload_running:
11231136
# If Patroni/PostgreSQL has not started yet and TLS relations was initialised,
11241137
# then mark TLS as enabled. This commonly happens when the charm is deployed
11251138
# in a bundle together with the TLS certificates operator. This flag is used to
11261139
# know when to call the Patroni API using HTTP or HTTPS.
11271140
self.unit_peer_data.update({"tls": "enabled" if self.is_tls_enabled else ""})
1141+
logger.debug("Early exit update_config: Workload not started yet")
1142+
return True
1143+
1144+
if not self._patroni.member_started:
11281145
logger.debug("Early exit update_config: Patroni not started yet")
1129-
return
1146+
return False
11301147

11311148
restart_postgresql = self.is_tls_enabled != self.postgresql.is_tls_enabled()
11321149
self._patroni.reload_patroni_configuration()
@@ -1140,6 +1157,8 @@ def update_config(self) -> None:
11401157
)
11411158
self.on[self.restart_manager.name].acquire_lock.emit()
11421159

1160+
return True
1161+
11431162
def _update_pebble_layers(self) -> None:
11441163
"""Update the pebble layers to keep the health check URL up-to-date."""
11451164
container = self.unit.get_container("postgresql")

0 commit comments

Comments
 (0)