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.

Sign up for GitHub

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

feat(providers): single database mode #1101

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat(providers): single database mode #1101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fc1907d
refactor(providers): const provutils.DefaultPGPort
vkrizan Dec 4, 2024
0ad3d69
refactor(providers): const provutils.DefaultPGAdminUsername
vkrizan Dec 4, 2024
50dcb4a
refactor(providers): introduce provutils.PGAdminConnectionStr
vkrizan Dec 6, 2024
970fc34
refactor(providers): introduce provutils.ReadDbConfigFromSecret
vkrizan Dec 9, 2024
030678d
feat(providers): single database mode
vkrizan Dec 6, 2024
7823169
test(kuttl): add test-multi-app-single-db
vkrizan Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions apis/cloud.redhat.com/v1alpha1/clowdenvironment_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,16 +284,19 @@ type KafkaConfig struct {
}

// DatabaseMode details the mode of operation of the Clowder Database Provider
// +kubebuilder:validation:Enum=shared;app-interface;local;none
// +kubebuilder:validation:Enum=single;shared;app-interface;local;none
type DatabaseMode string

// DatabaseConfig configures the Clowder provider controlling the creation of
// Database instances.
type DatabaseConfig struct {
// The mode of operation of the Clowder Database Provider. Valid options are:
// (*_app-interface_*) where the provider will pass through database credentials
// found in the secret defined by the database name in the ClowdApp, and (*_local_*)
// where the provider will spin up a local instance of the database.
// 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, (*_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.
Comment on lines +296 to +299
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Mode DatabaseMode `json:"mode"`

// Indicates where Clowder will fetch the database CA certificate bundle from. Currently only used in
Expand Down
13 changes: 8 additions & 5 deletions config/crd/bases/cloud.redhat.com_clowdenvironments.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,15 @@ spec:
type: string
mode:
description: 'The mode of operation of the Clowder Database
Provider. Valid options are: (*_app-interface_*) where the
provider will pass through database credentials found in
the secret defined by the database name in the ClowdApp,
and (*_local_*) where the provider will spin up a local
instance of the database.'
Provider. Valid options are: (*_app-interface_*) where
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
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the shared option is not documented here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hopefully fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

similar to local but keeps only one database deployment
and isolates apps by PG schemas.'
enum:
- single
- shared
- app-interface
- local
Expand Down
28 changes: 6 additions & 22 deletions controllers/cloud.redhat.com/providers/database/localdb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (db *localDbProvider) Provide(app *crd.ClowdApp) error {
dataInit := func() map[string]string {

hostname := fmt.Sprintf("%v.%v.svc", nn.Name, nn.Namespace)
port := "5432"
port := provutils.DefaultPGPort
username := utils.RandString(16)
name := app.Spec.Database.Name

Expand Down Expand Up @@ -116,7 +116,7 @@ func (db *localDbProvider) Provide(app *crd.ClowdApp) error {
if err != nil {
return errors.Wrap("couldn't convert to int", err)
}
dbCfg.AdminUsername = "postgres"
dbCfg.AdminUsername = provutils.DefaultPGAdminUsername
dbCfg.SslMode = "disable"

var image string
Expand Down Expand Up @@ -183,39 +183,23 @@ func (db *localDbProvider) processSharedDB(app *crd.ClowdApp) error {
return err
}

dbCfg := config.DatabaseConfig{}
dbCfg.SslMode = "disable"

refApp, err := crd.GetAppForDBInSameEnv(db.Ctx, db.Client, app)

if err != nil {
return err
}

secret := core.Secret{}

dbCfg := config.DatabaseConfig{}
inn := types.NamespacedName{
Name: fmt.Sprintf("%s-db", refApp.Name),
Namespace: refApp.Namespace,
}

// This is a REAL call here, not a cached call as the reconciliation must have been processed
// for the app we depend on.
if err = db.Client.Get(db.Ctx, inn, &secret); err != nil {
return errors.Wrap("Couldn't set/get secret", err)
}

secMap := make(map[string]string)

for k, v := range secret.Data {
(secMap)[k] = string(v)
}

err = dbCfg.Populate(&secMap)
// for the app we depend on, hence the nil for the ident.
err = provutils.ReadDbConfigFromSecret(db.Provider, nil, &dbCfg, inn)
if err != nil {
return errors.Wrap("couldn't convert to int", err)
return err
}
dbCfg.AdminUsername = "postgres"

db.Config.Database = &dbCfg

Expand Down
2 changes: 2 additions & 0 deletions controllers/cloud.redhat.com/providers/database/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ var imageList map[int32]string
func GetDatabase(c *p.Provider) (p.ClowderProvider, error) {
dbMode := c.Env.Spec.Providers.Database.Mode
switch dbMode {
case "single":
return NewSingleDBProvider(c)
case "shared":
return NewSharedDBProvider(c)
case "local":
Expand Down
42 changes: 11 additions & 31 deletions controllers/cloud.redhat.com/providers/database/shareddb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func createVersionedDatabase(p *providers.Provider, version int32) (*config.Data
dataInit := func() map[string]string {
return map[string]string{
"hostname": fmt.Sprintf("%v.%v.svc", nn.Name, nn.Namespace),
"port": "5432",
"port": provutils.DefaultPGPort,
"username": utils.RandString(16),
"password": password,
"pgPass": pgPassword,
Expand All @@ -103,7 +103,7 @@ func createVersionedDatabase(p *providers.Provider, version int32) (*config.Data
if err != nil {
return nil, errors.Wrap("couldn't convert to int", err)
}
dbCfg.AdminUsername = "postgres"
dbCfg.AdminUsername = provutils.DefaultPGAdminUsername
dbCfg.SslMode = "disable"

var image string
Expand Down Expand Up @@ -243,7 +243,7 @@ func (db *sharedDbProvider) Provide(app *crd.ClowdApp) error {
return err
}

dbCfg.AdminUsername = "postgres"
dbCfg.AdminUsername = provutils.DefaultPGAdminUsername
dbCfg.AdminPassword = string(vSec.Data["pgPass"])
dbCfg.Hostname = string(vSec.Data["hostname"])
dbCfg.Name = app.Spec.Database.Name
Expand All @@ -252,12 +252,8 @@ func (db *sharedDbProvider) Provide(app *crd.ClowdApp) error {
dbCfg.Port = int(port)
dbCfg.SslMode = "disable"

host := dbCfg.Hostname
user := dbCfg.AdminUsername
password := dbCfg.AdminPassword
dbname := app.Spec.Database.Name

appSQLConnectionString := fmt.Sprintf("host=%s port=%d user=%s password=%s dbname=%s sslmode=disable", host, port, user, password, dbname)
appSQLConnectionString := provutils.PGAdminConnectionStr(&dbCfg, dbname)

ctx, cancel := context.WithTimeout(db.Ctx, 5*time.Second)
defer cancel()
Expand All @@ -273,8 +269,7 @@ func (db *sharedDbProvider) Provide(app *crd.ClowdApp) error {
if pErr != nil {
if strings.Contains(pErr.Error(), fmt.Sprintf("database \"%s\" does not exist", app.Spec.Database.Name)) {

envSQLConnectionString := fmt.Sprintf("host=%s port=%d user=%s password=%s dbname=%s sslmode=disable", host, port, user, password, db.Env.Name)

envSQLConnectionString := provutils.PGAdminConnectionStr(&dbCfg, db.Env.Name)
envDbClient, envErr := sql.Open("postgres", envSQLConnectionString)
if envErr != nil {
return envErr
Expand Down Expand Up @@ -307,8 +302,8 @@ func (db *sharedDbProvider) Provide(app *crd.ClowdApp) error {
}

secret.StringData = map[string]string{
"hostname": host,
"port": "5432",
"hostname": dbCfg.Hostname,
"port": provutils.DefaultPGPort,
"username": dbCfg.Username,
"password": dbCfg.Password,
"pgPass": dbCfg.AdminPassword,
Expand Down Expand Up @@ -337,39 +332,24 @@ func (db *sharedDbProvider) processSharedDB(app *crd.ClowdApp) error {
return err
}

dbCfg := config.DatabaseConfig{}
dbCfg.SslMode = "disable"

refApp, err := crd.GetAppForDBInSameEnv(db.Ctx, db.Client, app)

if err != nil {
return err
}

secret := core.Secret{}

dbCfg := config.DatabaseConfig{}
inn := types.NamespacedName{
Name: fmt.Sprintf("%s-db", refApp.Name),
Namespace: refApp.Namespace,
}

// This is a REAL call here, not a cached call as the reconciliation must have been processed
// for the app we depend on.
if err = db.Client.Get(db.Ctx, inn, &secret); err != nil {
return errors.Wrap("Couldn't set/get secret", err)
}

secMap := make(map[string]string)

for k, v := range secret.Data {
(secMap)[k] = string(v)
}

err = dbCfg.Populate(&secMap)
// for the app we depend on, hence the nil for the ident.
err = provutils.ReadDbConfigFromSecret(db.Provider, nil, &dbCfg, inn)
if err != nil {
return errors.Wrap("couldn't convert to int", err)
return err
}
dbCfg.AdminUsername = "postgres"

db.Config.Database = &dbCfg

Expand Down
Loading
Loading