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-2740] Handle tables ownership #298

Merged
merged 1 commit into from Nov 30, 2023

Conversation

marceloneppel
Copy link
Member

@marceloneppel marceloneppel commented Nov 29, 2023

Issue

Some charms like Discourse-K8S need to have the ownership of the tables and other objects in a database to correctly run its migrations if a database from a previous Discourse installation is used, otherwise it will not complete the migrations and will throw an error. Also, the existing Discourse K8S test was marked as unstable in the past because that charm was migrated from the legacy to the modern relation interface.

Solution

This is a port of canonical/postgresql-k8s-operator#334.
lib/charms/postgresql_k8s/v0/postgresql.py is copied from the K8S charm.

If the relation between PostgreSQL and Discourse K8S (or any other charm), continue with the same behaviour, but after another instance of the charm is related to the PostgreSQL again, requesting the same database, check if there is no other application related and that previously requested the same database. If so, change the ownership of the objects in the database to the new user created for this relation. If there is already another application related to this database, just give permission to the database object, as we are already doing.

I manually tested these changes in a cross-model relation between this charm and Discourse-K8S.

Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
@marceloneppel marceloneppel marked this pull request as ready for review November 29, 2023 20:26
Comment on lines +318 to +333
sql.SQL(
"""DO $$
DECLARE r RECORD;
BEGIN
FOR r IN (SELECT statement FROM (SELECT 1 AS index,'ALTER TABLE '|| schemaname || '."' || tablename ||'" OWNER TO {};' AS statement
FROM pg_tables WHERE NOT schemaname IN ('pg_catalog', 'information_schema')
UNION SELECT 2 AS index,'ALTER SEQUENCE '|| sequence_schema || '."' || sequence_name ||'" OWNER TO {};' AS statement
FROM information_schema.sequences WHERE NOT sequence_schema IN ('pg_catalog', 'information_schema')
UNION SELECT 3 AS index,'ALTER FUNCTION '|| nsp.nspname || '."' || p.proname ||'"('||pg_get_function_identity_arguments(p.oid)||') OWNER TO {};' AS statement
FROM pg_proc p JOIN pg_namespace nsp ON p.pronamespace = nsp.oid WHERE NOT nsp.nspname IN ('pg_catalog', 'information_schema')
UNION SELECT 4 AS index,'ALTER VIEW '|| schemaname || '."' || viewname ||'" OWNER TO {};' AS statement
FROM pg_catalog.pg_views WHERE NOT schemaname IN ('pg_catalog', 'information_schema')) AS statements ORDER BY index) LOOP
EXECUTE format(r.statement);
END LOOP;
END; $$;"""
).format(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit read-only... and looks crazy... LGTM, but we should think on background about possible simplification here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks Alex. I agree. I created https://warthogs.atlassian.net/browse/DPE-3090 for that.

@marceloneppel marceloneppel merged commit 3ceaeeb into main Nov 30, 2023
38 checks passed
@marceloneppel marceloneppel deleted the dpe-2740-handle-tables-ownership branch November 30, 2023 11:33
BON4 pushed a commit to BON4/postgresql-operator that referenced this pull request Apr 23, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants