Handler errors (#5)

* Do not return error in handlers that is not an error in controller

* Do not return errors in user provisioning that are not controller errors

Author: Ivan Ivic

controllers/handler.go

@@ -8,7 +8,6 @@ import (
 	"github.com/doodlescheduling/k8sdb-controller/api/v1beta1"
 	infrav1beta1 "github.com/doodlescheduling/k8sdb-controller/api/v1beta1"
 	"github.com/doodlescheduling/k8sdb-controller/common/db"
-	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -71,7 +70,7 @@ func extractCredentials(credentials *infrav1beta1.SecretReference, secret *corev
 	return user, pw, nil
 }
 
-func reconcileDatabase(ctx context.Context, c client.Client, pool *db.ClientPool, invoke db.Invoke, database database, logger logr.Logger, recorder record.EventRecorder) (database, ctrl.Result, error) {
+func reconcileDatabase(c client.Client, pool *db.ClientPool, invoke db.Invoke, database database, recorder record.EventRecorder) (database, ctrl.Result, error) {
 	// Fetch referencing root secret
 	secret := &corev1.Secret{}
 	secretName := types.NamespacedName{
@@ -85,7 +84,7 @@ func reconcileDatabase(ctx context.Context, c client.Client, pool *db.ClientPool
 		msg := fmt.Sprintf("Referencing root secret was not found: %s", err.Error())
 		recorder.Event(database, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(database, v1beta1.SecretNotFoundReason, msg)
-		return database, ctrl.Result{Requeue: true}, err
+		return database, ctrl.Result{Requeue: true}, nil
 	}
 
 	usr, pw, err := extractCredentials(database.GetRootSecret(), secret)
@@ -94,32 +93,32 @@ func reconcileDatabase(ctx context.Context, c client.Client, pool *db.ClientPool
 		msg := fmt.Sprintf("Credentials field not found in referenced rootSecret: %s", err.Error())
 		recorder.Event(database, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(database, infrav1beta1.CredentialsNotFoundReason, msg)
-		return database, ctrl.Result{Requeue: true}, err
+		return database, ctrl.Result{Requeue: true}, nil
 	}
 
 	dbHandler, err := pool.FromURI(context.TODO(), invoke, database.GetAddress(), usr, pw)
 	if err != nil {
 		msg := fmt.Sprintf("Failed to setup connection to database server: %s", err.Error())
 		recorder.Event(database, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(database, infrav1beta1.ConnectionFailedReason, msg)
-		return database, ctrl.Result{Requeue: true}, err
+		return database, ctrl.Result{Requeue: true}, nil
 	}
 
 	err = dbHandler.CreateDatabaseIfNotExists(database.GetDatabaseName())
 	if err != nil {
 		msg := fmt.Sprintf("Failed to provision database: %s", err.Error())
 		recorder.Event(database, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(database, infrav1beta1.CreateDatabaseFailedReason, msg)
-		return database, ctrl.Result{Requeue: true}, err
+		return database, ctrl.Result{Requeue: true}, nil
 	}
 
 	msg := "Database successfully provisioned"
 	recorder.Event(database, "Normal", "info", msg)
 	v1beta1.DatabaseReadyCondition(database, v1beta1.DatabaseProvisiningSuccessfulReason, msg)
-	return database, ctrl.Result{}, err
+	return database, ctrl.Result{}, nil
 }
 
-func reconcileUser(ctx context.Context, database database, c client.Client, pool *db.ClientPool, invoke db.Invoke, user user, logger logr.Logger, recorder record.EventRecorder) (user, ctrl.Result, error) {
+func reconcileUser(database database, c client.Client, pool *db.ClientPool, invoke db.Invoke, user user, recorder record.EventRecorder) (user, ctrl.Result, error) {
 	// Fetch referencing database
 	databaseName := types.NamespacedName{
 		Namespace: user.GetNamespace(),
@@ -132,7 +131,7 @@ func reconcileUser(ctx context.Context, database database, c client.Client, pool
 		msg := fmt.Sprintf("Referencing database was not found: %s", err.Error())
 		recorder.Event(user, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(user, v1beta1.DatabaseNotFoundReason, msg)
-		return user, ctrl.Result{Requeue: true}, err
+		return user, ctrl.Result{Requeue: true}, nil
 	}
 
 	// Fetch referencing root secret
@@ -148,7 +147,7 @@ func reconcileUser(ctx context.Context, database database, c client.Client, pool
 		msg := fmt.Sprintf("Referencing root secret was not found: %s", err.Error())
 		recorder.Event(user, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(user, v1beta1.SecretNotFoundReason, msg)
-		return user, ctrl.Result{Requeue: true}, err
+		return user, ctrl.Result{Requeue: true}, nil
 	}
 
 	usr, pw, err := extractCredentials(database.GetRootSecret(), secret)
@@ -157,7 +156,7 @@ func reconcileUser(ctx context.Context, database database, c client.Client, pool
 		msg := fmt.Sprintf("Credentials field not found in referenced rootSecret: %s", err.Error())
 		recorder.Event(user, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(user, infrav1beta1.CredentialsNotFoundReason, msg)
-		return user, ctrl.Result{Requeue: true}, err
+		return user, ctrl.Result{Requeue: true}, nil
 	}
 
 	dbHandler, err := pool.FromURI(context.TODO(), invoke, database.GetAddress(), usr, pw)
@@ -166,7 +165,7 @@ func reconcileUser(ctx context.Context, database database, c client.Client, pool
 		msg := fmt.Sprintf("Failed to setup connection to database server: %s", err.Error())
 		recorder.Event(user, "Normal", "error", msg)
 		infrav1beta1.DatabaseNotReadyCondition(user, infrav1beta1.ConnectionFailedReason, msg)
-		return user, ctrl.Result{Requeue: true}, err
+		return user, ctrl.Result{Requeue: true}, nil
 	}
 
 	// Fetch referencing credentials secret
@@ -183,19 +182,19 @@ func reconcileUser(ctx context.Context, database database, c client.Client, pool
 		msg := fmt.Sprintf("No credentials found to provision user account: %s", err.Error())
 		recorder.Event(user, "Normal", "error", msg)
 		infrav1beta1.UserNotReadyCondition(user, infrav1beta1.CredentialsNotFoundReason, msg)
-		return user, ctrl.Result{Requeue: true}, err
+		return user, ctrl.Result{Requeue: true}, nil
 	}
 
 	err = dbHandler.SetupUser(database.GetDatabaseName(), usr, pw)
 	if err != nil {
 		msg := fmt.Sprintf("Failed to provison user account: %s", err.Error())
 		recorder.Event(user, "Normal", "error", msg)
 		infrav1beta1.UserNotReadyCondition(user, infrav1beta1.ConnectionFailedReason, msg)
-		return user, ctrl.Result{Requeue: true}, err
+		return user, ctrl.Result{Requeue: true}, nil
 	}
 
 	msg := "User successfully provisioned"
 	recorder.Event(user, "Normal", "info", msg)
 	v1beta1.UserReadyCondition(user, v1beta1.UserProvisioningSuccessfulReason, msg)
-	return user, ctrl.Result{}, err
+	return user, ctrl.Result{}, nil
 }

controllers/mongodbdatabase_controller.go

@@ -130,7 +130,7 @@ func (r *MongoDBDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return reconcile.Result{}, nil
 	}
 
-	_, result, reconcileErr := reconcileDatabase(ctx, r.Client, r.ClientPool, db.NewMongoDBServer, &database, logger, r.Recorder)
+	_, result, reconcileErr := reconcileDatabase(r.Client, r.ClientPool, db.NewMongoDBServer, &database, r.Recorder)
 
 	// Update status after reconciliation.
 	if err := r.patchStatus(ctx, &database); err != nil {

controllers/mongodbuser_controller.go

@@ -177,7 +177,7 @@ func (r *MongoDBUserReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}*/
 
 	var database infrav1beta1.MongoDBDatabase
-	_, result, reconcileErr := reconcileUser(ctx, &database, r.Client, r.ClientPool, db.NewMongoDBServer, &user, logger, r.Recorder)
+	_, result, reconcileErr := reconcileUser(&database, r.Client, r.ClientPool, db.NewMongoDBServer, &user, r.Recorder)
 
 	// Update status after reconciliation.
 	if err := r.patchStatus(ctx, &user); err != nil {

controllers/postgresqldatabase_controller.go

@@ -130,7 +130,7 @@ func (r *PostgreSQLDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return reconcile.Result{}, nil
 	}
 
-	_, result, reconcileErr := reconcileDatabase(ctx, r.Client, r.ClientPool, db.NewPostgreSQLServer, &database, logger, r.Recorder)
+	_, result, reconcileErr := reconcileDatabase(r.Client, r.ClientPool, db.NewPostgreSQLServer, &database, r.Recorder)
 
 	// Update status after reconciliation.
 	if err := r.patchStatus(ctx, &database); err != nil {

controllers/postgresqluser_controller.go

@@ -177,7 +177,7 @@ func (r *PostgreSQLUserReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	}*/
 
 	var database infrav1beta1.PostgreSQLDatabase
-	_, result, reconcileErr := reconcileUser(ctx, &database, r.Client, r.ClientPool, db.NewPostgreSQLServer, &user, logger, r.Recorder)
+	_, result, reconcileErr := reconcileUser(&database, r.Client, r.ClientPool, db.NewPostgreSQLServer, &user, r.Recorder)
 
 	// Update status after reconciliation.
 	if err := r.patchStatus(ctx, &user); err != nil {