Merge pull request #162 from kloeckner-i/METAL-3782/drop_schema_event

METAL-3782 better indication why database creation failed

Author: hyunysmile

controllers/database_controller.go

@@ -190,7 +190,6 @@ func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		case dbPhaseCreate:
 			err := r.createDatabase(ctx, dbcr)
 			if err != nil {
-				logrus.Errorf("DB: namespace=%s, name=%s failed creating database - %s", dbcr.Namespace, dbcr.Name, err)
 				// when database creation failed, don't requeue request. to prevent exceeding api limit (ex: against google api)
 				return r.manageError(ctx, dbcr, err, false)
 			}
@@ -353,7 +352,7 @@ func (r *DatabaseReconciler) createDatabase(ctx context.Context, dbcr *kciv1alph
 
 	err = database.Create(db, adminCred)
 	if err != nil {
-		logrus.Errorf("DB: namespace=%s, name=%s failed creating database", dbcr.Namespace, dbcr.Name)
+
 		return err
 	}
 

controllers/database_helper.go

@@ -74,17 +74,14 @@ func determinDatabaseType(dbcr *kciv1alpha1.Database, dbCred database.Credential
 	switch engine {
 	case "postgres":
 		extList := dbcr.Spec.Extensions
-		if monitoringEnabled {
-			extList = append(dbcr.Spec.Extensions, "pg_stat_statements")
-		}
-
 		db := database.Postgres{
 			Backend:          backend,
 			Host:             host,
 			Port:             uint16(port),
 			Database:         dbCred.Name,
 			User:             dbCred.Username,
 			Password:         dbCred.Password,
+			Monitoring:       monitoringEnabled,
 			Extensions:       extList,
 			SSLEnabled:       instance.Spec.SSLConnection.Enabled,
 			SkipCAVerify:     instance.Spec.SSLConnection.SkipVerify,

controllers/database_helper_test.go

@@ -110,14 +110,7 @@ func TestMonitoringEnabled(t *testing.T) {
 	db, _ := determinDatabaseType(postgresDbCr, testDbcred)
 	postgresInterface, _ := db.(database.Postgres)
 
-	found := false
-	for _, ext := range postgresInterface.Extensions {
-		if ext == "pg_stat_statements" {
-			found = true
-			break
-		}
-	}
-	assert.Equal(t, found, true, "expected pg_stat_statement is included in extension list")
+	assert.Equal(t, postgresInterface.Monitoring, true, "expected monitoring is true in postgres interface")
 }
 
 func TestPsqlDefaultConnectionStringGeneratationWithProxy(t *testing.T) {

pkg/utils/database/postgres.go

@@ -41,6 +41,7 @@ type Postgres struct {
 	Database         string
 	User             string
 	Password         string
+	Monitoring       bool
 	Extensions       []string
 	SSLEnabled       bool
 	SkipCAVerify     bool
@@ -161,10 +162,21 @@ func (p Postgres) createDatabase(admin AdminCredentials) error {
 		}
 	}
 
+	if p.Monitoring {
+		err := p.enableMonitoring(admin)
+		if err != nil {
+			return fmt.Errorf("can not enable monitoring - %s", err)
+		}
+	}
+
+	err := p.addExtensions(admin)
+	if err != nil {
+		return fmt.Errorf("can not add extension - %s", err)
+	}
+
 	if p.DropPublicSchema {
 		if err := p.dropPublicSchema(admin); err != nil {
-			logrus.Errorf("failed dropping the public schema %s", err)
-			return err
+			return fmt.Errorf("can not drop public schema - %s", err)
 		}
 		if len(p.Schemas) == 0 {
 			logrus.Info("the public schema is dropped, but no additional schemas are created, schema creation must be handled on the application side now")
@@ -178,12 +190,6 @@ func (p Postgres) createDatabase(admin AdminCredentials) error {
 		}
 	}
 
-	err := p.addExtensions(admin)
-	if err != nil {
-		logrus.Errorf("failed creating postgres extensions %s", err)
-		return err
-	}
-
 	return nil
 }
 
@@ -223,6 +229,10 @@ func (p Postgres) createUser(admin AdminCredentials) error {
 }
 
 func (p Postgres) dropPublicSchema(admin AdminCredentials) error {
+	if p.Monitoring {
+		return fmt.Errorf("can not drop public schema when monitoring is enabled on instance level")
+	}
+
 	drop := "DROP SCHEMA IF EXISTS public;"
 	if err := p.executeExec(p.Database, drop, admin); err != nil {
 		logrus.Errorf("failed to drop the schema Public: %s", err)
@@ -321,13 +331,24 @@ func (p Postgres) addExtensions(admin AdminCredentials) error {
 		query := fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS \"%s\";", ext)
 		err := p.executeExec(p.Database, query, admin)
 		if err != nil {
-			logrus.Errorf("failed creating extensions %s - %s", query, err)
 			return err
 		}
 	}
 	return nil
 }
 
+func (p Postgres) enableMonitoring(admin AdminCredentials) error {
+	monitoringExtension := "pg_stat_statements"
+
+	query := fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS \"%s\";", monitoringExtension)
+	err := p.executeExec(p.Database, query, admin)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (p Postgres) checkExtensions() error {
 	for _, ext := range p.Extensions {
 		query := fmt.Sprintf("SELECT 1 FROM pg_extension WHERE extname = '%s';", ext)

pkg/utils/database/postgres_test.go

@@ -24,7 +24,20 @@ import (
 )
 
 func testPostgres() *Postgres {
-	return &Postgres{"local", test.GetPostgresHost(), test.GetPostgresPort(), "testdb", "testuser", "testpassword", []string{}, false, false, false, []string{}}
+	return &Postgres{
+		Backend:          "local",
+		Host:             test.GetPostgresHost(),
+		Port:             test.GetPostgresPort(),
+		Database:         "testdb",
+		User:             "testuser",
+		Password:         "testpassword",
+		Monitoring:       false,
+		Extensions:       []string{},
+		SSLEnabled:       false,
+		SkipCAVerify:     false,
+		DropPublicSchema: false,
+		Schemas:          []string{},
+	}
 }
 
 func getPostgresAdmin() AdminCredentials {
@@ -78,21 +91,41 @@ func TestDropPublicSchemaFail(t *testing.T) {
 	assert.Error(t, p.checkSchemas())
 }
 
-func TestDropPublicSchema(t *testing.T) {
+func TestDropPublicSchemaMonitoringTrue(t *testing.T) {
 	p := testPostgres()
 	admin := getPostgresAdmin()
+	p.Monitoring = true
+	p.DropPublicSchema = true
+	p.dropPublicSchema(admin)
+	assert.Error(t, p.checkSchemas())
+}
+
+func TestDropPublicSchemaMonitoringFalse(t *testing.T) {
+	p := testPostgres()
+	admin := getPostgresAdmin()
+	p.Monitoring = false
 	p.DropPublicSchema = true
 	p.dropPublicSchema(admin)
 	assert.NoError(t, p.checkSchemas())
 
-	// Schemas is recreated here not to breaks tests for extensions
+	// Schemas is recreated here not to breaks tests
 	p.Schemas = []string{"public"}
 	assert.NoError(t, p.createSchemas(admin))
 }
 
+func TestEnableMonitoring(t *testing.T) {
+	p := testPostgres()
+	admin := getPostgresAdmin()
+	p.Monitoring = true
+	p.enableMonitoring(admin)
+	p.Extensions = []string{"pg_stat_statements"}
+	assert.NoError(t, p.checkExtensions())
+}
+
 func TestPostgresNoExtensions(t *testing.T) {
 	admin := getPostgresAdmin()
 	p := testPostgres()
+	p.Extensions = []string{}
 
 	assert.NoError(t, p.addExtensions(admin))
 	assert.NoError(t, p.checkExtensions())