feat(providers): single database mode #1101

vkrizan · 2024-12-09T16:29:51Z

Creates a new database provider mode named single.
The aim of the mode is to create a single database instance and share it
between the apps by separating them by Posgres schemas.
Each app gets a schema created by the name of the requested database.
Schemas are created under environment's database, with the same name as
the ClowdEnv.

Adds the following refactorings:

provutils.DefaultPGPort to set port constant
provutils.DefaultPGAdminUsername to set default PG admin user
provutils.PGAdminConnectionStr that builds sanitized connection string to PG from a DatabaseConfig using admin creds
provutils.ReadDbConfigFromSecret to reuse read of DatabaseConfigs from secrets, optionally using a cache

RHINENG-14526

vkrizan · 2024-12-09T16:31:58Z

@psav I've opted for "single" as the name of the db mode. Suggestions and reviews are welcome. Thank you.

vkrizan · 2025-01-13T14:30:47Z

controllers/cloud.redhat.com/providers/database/single.go

+
+	var pgPassword string
+	if setAdmin {
+		pgPassword, err = utils.RandPassword(16, provutils.RCharSet)


My initial thought was to disable admin user for apps, however some apps might still require this to do some setups (i.e. create additional users). @psav any suggestions how we should approach this situation? I guess, one of the option is to update those services needing such privileges, or look at the privileges case-by-case.

It's tricky because ephemeral usage often relies on them needing to do more stuff than usual, but in prod they will just get the credentials which are appropriate

There are some services that run a CREATE EXTENSION in their migrations ... for example advisor-backend uses the 'hll' extension. I'm not sure how something like that will work with 'single' DB mode. Perhaps we could have Clowder pre-load known extensions after it spins up the DB. Anyways, unless you need support for that right now, that is something a follow-up PR could address.

Yeah. I want to test those apps to see how they would work with this limited access... AFAIK Advisor, has dropped or is dropping the HLL extension.

We plan to remove use of the HLL extension next week (17th Feb 2025).

That's the only extension I'm aware of in Advisor.

vkrizan · 2025-01-13T14:33:53Z

I'm opening this up from a draft state for review. There still might be some small issues in certain conditions. I prefer to get this out sooner to do a bunch of testing on a test cluster. Thank you.

psav · 2025-01-13T15:40:20Z

I'm opening this up from a draft state for review. There still might be some small issues in certain conditions. I prefer to get this out sooner to do a bunch of testing on a test cluster. Thank you.

We should talk about the expectation here and I was thinking we can deploy this for testing in the cluster - without merging it and only merge when we are ready - allows us to not goof up the cluster too much. @bsquizz @wcmitchell

adamrdrew · 2025-01-28T13:35:32Z

/retest

adamrdrew · 2025-01-28T18:35:05Z

/retest

PaulWay

I don't know Go or Clowder well enough to know whether it works or not but just contributing a few suggestions for the documentation.

PaulWay · 2025-01-28T21:26:54Z

config/crd/bases/cloud.redhat.com_clowdenvironments.yaml

+                          the provider will pass through database credentials found
+                          in the secret defined by the database name in the
+                          ClowdApp, (*_local_*) where the provider will spin up a
+                          local instance of the database, and (*_single_*) that is


It looks like the shared option is not documented here.

hopefully fixed

I think another make pre-push will be needed to get the above changes to the comment reflected in the description here.

I've called it again, and there is no visible change.

I've used this as the no-update target is git setup dependent:

make manifests generate genconfig build-template api-docs

docs/providers/database.md

bsquizz

The approach looks good and I like the consolidation of common operations into provutils. Aside from my nitpicks on the DB mode descriptions, I think the biggest thing this needs is a kuttl test to ensure this works as expected. Let me know if you need help to get started on creating one.

bsquizz · 2025-02-07T20:46:43Z

apis/cloud.redhat.com/v1alpha1/clowdenvironment_types.go

+	// where the provider will spin up a local instance of the database, (*_shared_*)
+	// where the provider will spin up only one shared instance per requested
+	// PostgreSQL version, and (*_single_*) that is similar to local and shared modes
+	// but keeps only one database deployment and isolates apps by PG schemas.


Sorry to pick on shared mode since its not original intent of your PR to mess with this... I think the intent of it is to have 2+ ClowdApps share the same DB (same database, username, and password) correct?

The difference between 'shared' and 'single' can be confusing. Would wording like this clarify it better?

Suggested change

// where the provider will spin up a local instance of the database, (*_shared_*)

// where the provider will spin up only one shared instance per requested

// PostgreSQL version, and (*_single_*) that is similar to local and shared modes

// but keeps only one database deployment and isolates apps by PG schemas.

// where the provider will spin up a local instance of the database, (*_shared_*)

// where the provider will spin up one instance of the DB server intended to be

// accessed by multiple ClowdApps using the same database, and (*_single_*)

// which spins up one instance of the DB server and isolates each ClowdApp's

// access by creating a schema per ClowdApp.

bsquizz · 2025-02-07T20:48:23Z

config/crd/bases/cloud.redhat.com_clowdenvironments.yaml

+                          the provider will pass through database credentials found
+                          in the secret defined by the database name in the
+                          ClowdApp, (*_local_*) where the provider will spin up a
+                          local instance of the database, and (*_single_*) that is


I think another make pre-push will be needed to get the above changes to the comment reflected in the description here.

bsquizz · 2025-02-07T20:51:26Z

deploy-mutate.yml

-                            instance of the database.'
+                            (*_local_*) where the provider will spin up a local instance
+                            of the database, and (*_single_*) that is similar to local
+                            but keeps only one database deployment and isolates apps


make pre-push will likely update this

bsquizz · 2025-02-07T20:51:38Z

deploy.yml

-                            instance of the database.'
+                            (*_local_*) where the provider will spin up a local instance
+                            of the database, and (*_single_*) that is similar to local
+                            but keeps only one database deployment and isolates apps


bsquizz · 2025-02-07T20:53:26Z

controllers/cloud.redhat.com/providers/database/single.go

+
+	var pgPassword string
+	if setAdmin {
+		pgPassword, err = utils.RandPassword(16, provutils.RCharSet)


There are some services that run a CREATE EXTENSION in their migrations ... for example advisor-backend uses the 'hll' extension. I'm not sure how something like that will work with 'single' DB mode. Perhaps we could have Clowder pre-load known extensions after it spins up the DB. Anyways, unless you need support for that right now, that is something a follow-up PR could address.

Creates a new database provider mode named `single`. The aim of the mode is to create a single database instance and share it between the apps by separating them by Posgres schemas. Each app gets a schema created by the name of the requested database. Schemas are created under environment's database, with the same name as the ClowdEnv. RHINENG-14526

vkrizan · 2025-02-13T17:12:54Z

@bsquizz I've added a (blind) kuttl test. Blind, because I'm not able to run those locally. It's very heavy on resources.

vkrizan force-pushed the single-db-provider branch from 0f87516 to 9e4a061 Compare December 10, 2024 12:50

vkrizan force-pushed the single-db-provider branch 2 times, most recently from e0a2a56 to 86ee564 Compare December 20, 2024 14:44

vkrizan force-pushed the single-db-provider branch from 86ee564 to 5d02dad Compare January 13, 2025 14:18

vkrizan commented Jan 13, 2025

View reviewed changes

vkrizan marked this pull request as ready for review January 13, 2025 14:33

vkrizan force-pushed the single-db-provider branch from 5d02dad to b7285df Compare January 13, 2025 14:55

PaulWay reviewed Jan 28, 2025

View reviewed changes

vkrizan force-pushed the single-db-provider branch 2 times, most recently from 7e86346 to e270e2a Compare February 3, 2025 15:23

bsquizz requested changes Feb 7, 2025

View reviewed changes

vkrizan added 6 commits February 13, 2025 18:07

refactor(providers): const provutils.DefaultPGPort

fc1907d

refactor(providers): const provutils.DefaultPGAdminUsername

0ad3d69

refactor(providers): introduce provutils.PGAdminConnectionStr

50dcb4a

refactor(providers): introduce provutils.ReadDbConfigFromSecret

970fc34

test(kuttl): add test-multi-app-single-db

7823169

vkrizan force-pushed the single-db-provider branch from e270e2a to 7823169 Compare February 13, 2025 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(providers): single database mode #1101

feat(providers): single database mode #1101

vkrizan commented Dec 9, 2024

vkrizan commented Dec 9, 2024

vkrizan Jan 13, 2025

psav Jan 13, 2025

bsquizz Feb 7, 2025

vkrizan Feb 10, 2025

PaulWay Feb 13, 2025

PaulWay Feb 13, 2025

vkrizan commented Jan 13, 2025

psav commented Jan 13, 2025

adamrdrew commented Jan 28, 2025

adamrdrew commented Jan 28, 2025

PaulWay left a comment

PaulWay Jan 28, 2025

vkrizan Feb 3, 2025

bsquizz Feb 7, 2025

vkrizan Feb 13, 2025

bsquizz left a comment

bsquizz Feb 7, 2025

bsquizz Feb 7, 2025

bsquizz Feb 7, 2025

bsquizz Feb 7, 2025

bsquizz Feb 7, 2025

vkrizan commented Feb 13, 2025

feat(providers): single database mode #1101

Are you sure you want to change the base?

feat(providers): single database mode #1101

Conversation

vkrizan commented Dec 9, 2024

vkrizan commented Dec 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkrizan commented Jan 13, 2025

psav commented Jan 13, 2025

adamrdrew commented Jan 28, 2025

adamrdrew commented Jan 28, 2025

PaulWay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsquizz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkrizan commented Feb 13, 2025