Add PostgreSQL primary support to the kosmos-ejabberd cookbook #181

Merged
greg merged 6 commits from feature/180-ejabberd_pg_primary into master 2020-06-19 14:46:52 +00:00
Owner
  • Move the PostgreSQL user and database creation to a pg_db recipe
  • Generate access rights for the ejabberd servers in the pg_db recipe
  • Connect to the PostgreSQL primary instead of localhost

Refs #180

* Move the PostgreSQL user and database creation to a `pg_db` recipe * Generate access rights for the ejabberd servers in the `pg_db` recipe * Connect to the PostgreSQL primary instead of localhost Refs #180
greg self-assigned this 2020-06-10 16:45:44 +00:00
Owner

I find it rather complicated that we should specifically allow PostgreSQL access per app and database, instead of allowing our own servers to connect to the primary by default. Why wouldn't a replica be allowed to connect to the primary just because it's a replica? It does that for replication anyway, and thus should needs access to all databases in the first place.

The fact that configuring simple database access requires an entire recipe should ring some alarm bells in my opinion.

I find it rather complicated that we should specifically allow PostgreSQL access per app and database, instead of allowing our own servers to connect to the primary by default. Why wouldn't a replica be allowed to connect to the primary just because it's a replica? It does that for replication anyway, and thus should needs access to all databases in the first place. The fact that configuring simple database access requires an entire recipe should ring some alarm bells in my opinion.
Author
Owner

Replication is a special right, this is defined here for replicas: 81403b7cb9/site-cookbooks/kosmos-postgresql/recipes/default.rb (L48-L55).

A server connecting to the PostgreSQL primary as a client is not necessarily a replica, but as you said it could be done for all databases at once. I had the idea of definining each database as we need a recipe anyway to create the postgreSQL user and database for each service. Allowing access to all databases for a server could be done as part of the primary recipe, however it means the list of all services that use PostgreSQL would live there instead of the recipe for the service. I'm not opposed to doing that, I just thought doing it per database was clearer.

Replication is a special right, this is defined here for replicas: https://gitea.kosmos.org/kosmos/chef/src/commit/81403b7cb9858c48cc9262d4b79f1a63f5766016/site-cookbooks/kosmos-postgresql/recipes/default.rb#L48-L55. A server connecting to the PostgreSQL primary as a client is not necessarily a replica, but as you said it could be done for all databases at once. I had the idea of definining each database as we need a recipe anyway to create the postgreSQL user and database for each service. Allowing access to all databases for a server could be done as part of the primary recipe, however it means the list of all services that use PostgreSQL would live there instead of the recipe for the service. I'm not opposed to doing that, I just thought doing it per database was clearer.
Owner

I didn't mean replication per se, but all Hetzner machines would definitely be replicas for now anyway.

I mean that any of our Hetzner machines should be able to connect to the Posgtgres server in the first place. The actual authentication is then the username/password for the database, and doesn't require whitelisting the host per database.

I didn't mean replication per se, but all Hetzner machines would definitely be replicas for now anyway. I mean that any of our Hetzner machines should be able to connect to the Posgtgres server in the first place. The actual authentication is then the username/password for the database, and doesn't require whitelisting the host per database.
Author
Owner

I'm going to look into it, there has to be a way that looks good in Chef code. Not having to run Chef again on the primary every time we add a new service will be useful

I'm going to look into it, there has to be a way that looks good in Chef code. Not having to run Chef again on the primary every time we add a new service will be useful
Owner

It's not about looking good, it's about adding unnecessary complexities and code.

It's not about looking good, it's about adding unnecessary complexities and code.
Author
Owner

Pushed 6f696d7, what do you think?

Pushed 6f696d7, what do you think?
Owner

At least it's half the LOC. Looks much better to me.

However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync.

At least it's half the LOC. Looks much better to me. However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync.
Author
Owner

However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync.

I have found a better way. See ee9c241a4d, I have added a postgresql_client role

> However, I don't quite understand why it still has to be searching for every single app. Isn't there an easier way to know that a machine has roles/attributes that require postgres access? If it's not bound to the application, then it can never be out of sync. I have found a better way. See ee9c241a4d, I have added a `postgresql_client` role
raucao approved these changes 2020-06-12 15:27:12 +00:00
raucao left a comment
Owner

LGTM now. 👍

LGTM now. :+1:
greg closed this pull request 2020-06-19 14:46:52 +00:00
greg deleted branch feature/180-ejabberd_pg_primary 2020-06-19 14:46:59 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: kosmos/chef#181
No description provided.