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-4453] Fix scale up with S3 and TLS relations #480

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).

Signed-off-by: Marcelo Henrique Neppel <[email protected]>
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! 1 minor comment here

@@ -512,6 +528,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? It feels more important than this to me, maybe info level? 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.

@marceloneppel marceloneppel merged commit 85f92e9 into main May 31, 2024
53 checks passed
@marceloneppel marceloneppel deleted the dpe-4453-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