[DPE-3349] Handle S3 relation in primary non-leader unit #375
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
…tanza-on-primary Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
dragomirp
reviewed
Feb 6, 2024
Comment on lines
+331
to
+334
| if self.charm.unit.is_leader(): | ||
| self.charm.app_peer_data.update( | ||
| {"stanza": self.stanza_name, "init-pgbackrest": "True"} | ||
| ) |
There was a problem hiding this comment.
If we have to check the unit databags anyway, would it make sense to not set the app data at all? that way we should be able to have a single flow using the unit data, instead of having to check two places.
There was a problem hiding this comment.
Good point. Let me try to recall if there is any reason for this approach.
There was a problem hiding this comment.
I remembered the reason I have chosen the approach with both application and unit relation databags: using only the unit relation databag, if we scale down the application and the primary unit is removed, we need to handle the transfer of the stanza name to one of the remaining units in something like the relation departed event or keep the stanza name in all the units, which seems not so good as keeping it in the application relation databag.
dragomirp
approved these changes
Feb 6, 2024
taurus-forever
approved these changes
Feb 8, 2024
…tanza-on-primary Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
BON4
pushed a commit
to BON4/postgresql-k8s-operator
that referenced
this pull request
May 20, 2024
* Handle S3 relation in primary non-leader unit Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Fix relation data removal Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Update integration test Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Fix existing unit tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Add extra unit tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Add unit test for peer relation changed event handler Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Handle pgBackRest service check when the primary was not elected yet Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Speedup the new check on test Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Add log call Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * Speed up events Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> --------- Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
The stanza can only be initialised in the PostgreSQL primary unit if it's the Juju leader unit.
Otherwise, the unit fails with an error, as shown in #370.
Solution
Allow the non-leader unit (when it's the PostgreSQL primary) to initialise the stanza.
For that, all checks for the unit being the Juju leader unit were changed to check whether the unit is the PostgreSQL primary.
Also, some coordination is now made in the relation databags to share the stanza name and the
init-pgbackrestflag.The current flow doesn't change if the PostgreSQL primary unit is also the Juju leader unit.
If they are different units, then:
init-pgbackresflag are set in the primary unit relation databag.pgbackrest checkcommand is executed in the primary unit, which clears theinit-pgbackrestflag from the unit relation databag.init-pgbackrestflag from the application relation databag.Both unit and integration tests were added/updated to ensure we keep this flow intact.
Also, a test for #273 was added through this PR, as it was also related to backups/restores.