Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MISC] Merge update_tls_flag into update_endpoints #669

Merged
merged 2 commits into from
Nov 12, 2024
Merged

[MISC] Merge update_tls_flag into update_endpoints #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38ccc28
Merge update_tls_flag into update_endpoints
dragomirp Nov 11, 2024
0eeb3b5
No peer data
dragomirp Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/charm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,8 @@ def _units_ips(self) -> set[str]:
@property
def members_ips(self) -> set[str]:
"""Returns the list of IPs addresses of the current members of the cluster."""
if not self._peers:
return set()
Comment on lines +825 to +826
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seen fail on ci:

   File "/var/lib/juju/agents/unit-postgresql-gcp-1/charm/./src/charm.py", line 825, in members_ips
    return set(json.loads(self._peers.data[self.app].get("members_ips", "[]")))
AttributeError: 'NoneType' object has no attribute 'data'

return set(json.loads(self._peers.data[self.app].get("members_ips", "[]")))

def _add_to_members_ips(self, ip: str) -> None:
Expand Down Expand Up @@ -1709,9 +1711,7 @@ def update_config(self, is_creating_backup: bool = False) -> bool:
# in a bundle together with the TLS certificates operator. This flag is used to
# know when to call the Patroni API using HTTP or HTTPS.
self.unit_peer_data.update({"tls": "enabled" if enable_tls else ""})
self.postgresql_client_relation.update_tls_flag(
"True" if self.is_tls_enabled else "False"
)
self.postgresql_client_relation.update_endpoints()
logger.debug("Early exit update_config: Workload not started yet")
return True

Expand Down Expand Up @@ -1787,7 +1787,7 @@ def _handle_postgresql_restart_need(self, enable_tls: bool) -> None:
# Ignore the error, as it happens only to indicate that the configuration has not changed.
pass
self.unit_peer_data.update({"tls": "enabled" if enable_tls else ""})
self.postgresql_client_relation.update_tls_flag("True" if self.is_tls_enabled else "False")
self.postgresql_client_relation.update_endpoints()

# Restart PostgreSQL if TLS configuration has changed
# (so the both old and new connections use the configuration).
Expand Down
30 changes: 8 additions & 22 deletions src/relations/postgresql_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,6 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
# Set the database name
self.database_provides.set_database(event.relation.id, database)

# Set TLS flag
self.database_provides.set_tls(
event.relation.id,
"True" if self.charm.is_tls_enabled else "False",
)

# Set TLS CA
if self.charm.is_tls_enabled:
_, ca, _ = self.charm.tls.get_tls_files()
self.database_provides.set_tls_ca(event.relation.id, ca)

Comment on lines -111 to -121
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Endpoints will be updated anyway

# Update the read/write and read-only endpoints.
self.update_endpoints(event)

Expand Down Expand Up @@ -201,6 +190,12 @@ def update_endpoints(self, event: DatabaseRequestedEvent = None) -> None:
else ""
)

tls = "True" if self.charm.is_tls_enabled else "False"
if tls == "True":
_, ca, _ = self.charm.tls.get_tls_files()
else:
ca = ""

for relation_id in rel_data:
user = f"relation-{relation_id}"
database = rel_data[relation_id].get("database")
Expand All @@ -226,17 +221,8 @@ def update_endpoints(self, event: DatabaseRequestedEvent = None) -> None:
f"postgresql://{user}:{password}@{self.charm.primary_endpoint}:{DATABASE_PORT}/{database}",
)

def update_tls_flag(self, tls: str) -> None:
"""Update TLS flag and CA in relation databag."""
relations = self.model.relations[self.relation_name]
if tls == "True":
_, ca, _ = self.charm.tls.get_tls_files()
else:
ca = ""

for relation in relations:
self.database_provides.set_tls(relation.id, tls)
self.database_provides.set_tls_ca(relation.id, ca)
self.database_provides.set_tls(relation_id, tls)
self.database_provides.set_tls_ca(relation_id, ca)

def _check_multiple_endpoints(self) -> bool:
"""Checks if there are relations with other endpoints."""
Expand Down
9 changes: 6 additions & 3 deletions tests/unit/test_postgresql_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ def test_on_database_requested(harness):
"password": "test-password",
"version": POSTGRESQL_VERSION,
"database": f"{DATABASE}",
"tls": "False",
}

# Assert no BlockedStatus was set.
Expand All @@ -154,7 +153,6 @@ def test_on_database_requested(harness):
# No data is set in the databag by the database.
assert harness.get_relation_data(rel_id, harness.charm.app.name) == {
"data": f'{{"database": "{DATABASE}", "extra-user-roles": "{EXTRA_USER_ROLES}"}}',
"tls": "False",
}

# BlockedStatus due to a PostgreSQLCreateDatabaseError.
Expand All @@ -163,7 +161,6 @@ def test_on_database_requested(harness):
# No data is set in the databag by the database.
assert harness.get_relation_data(rel_id, harness.charm.app.name) == {
"data": f'{{"database": "{DATABASE}", "extra-user-roles": "{EXTRA_USER_ROLES}"}}',
"tls": "False",
}

# BlockedStatus due to a PostgreSQLGetPostgreSQLVersionError.
Expand Down Expand Up @@ -256,6 +253,7 @@ def test_update_endpoints_with_event(harness):
"endpoints": "1.1.1.1:5432",
"read-only-endpoints": "2.2.2.2:5432",
"uris": "postgresql://relation-2:[email protected]:5432/test_db",
"tls": "False",
}
assert harness.get_relation_data(another_rel_id, harness.charm.app.name) == {}
_fetch_my_relation_data.assert_called_once_with([2], ["password"])
Expand All @@ -265,6 +263,7 @@ def test_update_endpoints_with_event(harness):
assert harness.get_relation_data(rel_id, harness.charm.app.name) == {
"endpoints": "1.1.1.1:5432",
"uris": "postgresql://relation-2:[email protected]:5432/test_db",
"tls": "False",
}
assert harness.get_relation_data(another_rel_id, harness.charm.app.name) == {}

Expand Down Expand Up @@ -331,11 +330,13 @@ def test_update_endpoints_without_event(harness):
"endpoints": "1.1.1.1:5432",
"read-only-endpoints": "2.2.2.2:5432",
"uris": "postgresql://relation-2:[email protected]:5432/test_db",
"tls": "False",
}
assert harness.get_relation_data(another_rel_id, harness.charm.app.name) == {
"endpoints": "1.1.1.1:5432",
"read-only-endpoints": "2.2.2.2:5432",
"uris": "postgresql://relation-3:[email protected]:5432/test_db2",
"tls": "False",
}
_fetch_my_relation_data.assert_called_once_with(None, ["password"])

Expand All @@ -344,8 +345,10 @@ def test_update_endpoints_without_event(harness):
assert harness.get_relation_data(rel_id, harness.charm.app.name) == {
"endpoints": "1.1.1.1:5432",
"uris": "postgresql://relation-2:[email protected]:5432/test_db",
"tls": "False",
}
assert harness.get_relation_data(another_rel_id, harness.charm.app.name) == {
"endpoints": "1.1.1.1:5432",
"uris": "postgresql://relation-3:[email protected]:5432/test_db2",
"tls": "False",
}
Loading