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.

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

[DPE-4456] Fix scale up with S3 and TLS relations #489

Merged
merged 1 commit into from
May 31, 2024

Conversation

marceloneppel
Copy link
Member

@marceloneppel marceloneppel commented May 28, 2024

Issue

We cannot scale up if S3 and TLS relations are established. This operation leads to an error like the one below if, in the new unit, the S3 parameters relation changed event fires before the TLS files are available in the unit (and Patroni + PostgreSQL haven't started yet).

requests.exceptions.SSLError: HTTPSConnectionPool(host='10.142.152.2', port=8008): Max retries exceeded with url: /cluster (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate in certificate chain (_ssl.c:1007)')))

Solution

Defer the S3 parameters relation changed event until that unit can do any operation related to S3 (or even connect to the other unit to check the cluster topology and verify it's not the primary unit so that it can abort any kind of stanza initialisation).

Port of canonical/postgresql-operator#480.

Signed-off-by: Marcelo Henrique Neppel <[email protected]>
@marceloneppel marceloneppel marked this pull request as ready for review May 29, 2024 10:34
Copy link
Member

@lucasgameiroborges lucasgameiroborges left a comment

Choose a reason for hiding this comment

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

LGTM! Left only one question.

@@ -455,6 +471,11 @@ def _on_s3_credential_changed(self, event: CredentialsChangedEvent):
logger.debug("Cannot set pgBackRest configurations, missing configurations.")
return

if not self._can_initialise_stanza:
logger.debug("Cannot initialise stanza yet.")
Copy link
Member

Choose a reason for hiding this comment

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

Should this be DEBUG level only? Kinda feel a little more important than this to me, but up to you!

Copy link
Member Author

Choose a reason for hiding this comment

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

I think DEBUG is okay for this case. Like the other messages we already have for similar situations, which don't provide immediate value for the user but only when troubleshooting, I think it's okay as it is.

Copy link
Contributor

@taurus-forever taurus-forever left a comment

Choose a reason for hiding this comment

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

LGTM, but 30 mins timeout scary me a lot...

@marceloneppel marceloneppel merged commit 7e8862e into main May 31, 2024
47 checks passed
@marceloneppel marceloneppel deleted the dpe-4456-fix-scale-up-with-s3-and-tls-relations branch May 31, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants