Skip to content

Commit d8ba7d6

Browse files
anbratenanbraten
committed
fix: correct reconcile loop and mongo db creation error
1 parent 0a173f3 commit d8ba7d6

File tree

2 files changed

+58
-94
lines changed

2 files changed

+58
-94
lines changed

‎adapters/mongo_adapter.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,14 @@ func (adapter mongoAdapter) HasDatabase(ctx context.Context, database string) (b
2222
return contains(databaseNames, database), err
2323
}
2424

25-
func (adapter mongoAdapter) CreateDatabase(ctx context.Context, name string) error {
26-
adapter.client.Database(name)
27-
return nil
25+
func (adapter mongoAdapter) CreateDatabase(ctx context.Context, database string) error {
26+
// create dummy data as mongo only creates databases if they contain something
27+
_, err := adapter.client.Database(database).Collection("__internal-placeholder__").InsertOne(ctx, bson.D{{Key: "empty", Value: true}})
28+
return err
2829
}
2930

30-
func (adapter mongoAdapter) DeleteDatabase(ctx context.Context, name string) error {
31-
return adapter.client.Database(name).Drop(ctx)
31+
func (adapter mongoAdapter) DeleteDatabase(ctx context.Context, database string) error {
32+
return adapter.client.Database(database).Drop(ctx)
3233
}
3334

3435
func (adapter mongoAdapter) HasDatabaseUserWithAccess(ctx context.Context, database string, username string) (bool, error) {

‎controllers/database_controller.go

+52-89
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const databaseFinalizer = "finalizer.database.anbraten.github.io"
3535
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
3636
func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
3737
log := r.Log.WithValues("database", req.NamespacedName)
38+
log.Info("Reconciling database")
3839

3940
// Fetch the Database instance
4041
database := &anbratengithubiov1alpha1.Database{}
@@ -54,58 +55,22 @@ func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
5455

5556
db, err := r.getDatabaseConnection(ctx, database.Spec.Type)
5657
if err != nil {
57-
log.Error(err, "Failed to detect and open database connection")
58+
log.Error(err, "Failed to detect or open database connection")
5859
return ctrl.Result{}, err
5960
}
6061

6162
defer db.Close(ctx)
6263

63-
log.Info("Connected to database server")
64-
65-
hasDatabase, err := db.HasDatabase(ctx, database.Spec.Database)
66-
if err != nil {
67-
log.Error(err, "Couldn't check if database exists")
68-
return ctrl.Result{}, err
69-
}
70-
71-
if !hasDatabase {
72-
log.Info("Create new database: '" + database.Spec.Database + "'")
73-
74-
err = db.CreateDatabase(ctx, database.Spec.Database)
75-
if err != nil {
76-
log.Error(err, "Can't create database")
77-
return ctrl.Result{}, err
78-
}
79-
}
80-
81-
hasDatabaseUserWithAccess, err := db.HasDatabaseUserWithAccess(ctx, database.Spec.Database, database.Spec.Username)
82-
if err != nil {
83-
log.Error(err, "Can't check if user has access to database")
84-
return ctrl.Result{}, err
85-
}
86-
87-
if !hasDatabaseUserWithAccess {
88-
log.Info("Create new user '" + database.Spec.Username + "' with access to the database '" + database.Spec.Database + "'")
89-
90-
err = db.CreateDatabaseUser(ctx, database.Spec.Database, database.Spec.Username, database.Spec.Password)
91-
if err != nil {
92-
log.Error(err, "Can't create database user with access to database")
93-
return ctrl.Result{}, err
94-
}
95-
}
96-
97-
log.Info("Created database and user with full access to it")
98-
9964
// Check if the Database instance is marked to be deleted, which is
10065
// indicated by the deletion timestamp being set.
10166
isDatabaseMarkedToBeDeleted := database.GetDeletionTimestamp() != nil
10267
if isDatabaseMarkedToBeDeleted {
103-
if contains(database.GetFinalizers(), databaseFinalizer) {
68+
if controllerutil.ContainsFinalizer(database, databaseFinalizer) {
10469
// Run finalization logic for databaseFinalizer. If the
10570
// finalization logic fails, don't remove the finalizer so
10671
// that we can retry during the next reconciliation.
107-
if err := r.finalizeDatabase(ctx, log, database); err != nil {
108-
log.Error(err, "Can't create finalizer for database")
72+
if err := r.finalizeDatabase(ctx, log, db, database); err != nil {
73+
log.Error(err, "Can't remove database and user")
10974
return ctrl.Result{}, err
11075
}
11176

@@ -114,19 +79,52 @@ func (r *DatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
11479
controllerutil.RemoveFinalizer(database, databaseFinalizer)
11580
err := r.Update(ctx, database)
11681
if err != nil {
117-
log.Error(err, "Can't remove finalizers of database")
11882
return ctrl.Result{}, err
11983
}
12084
}
12185
return ctrl.Result{}, nil
12286
}
12387

124-
// Add finalizer for this CR
125-
if !contains(database.GetFinalizers(), databaseFinalizer) {
126-
if err := r.addFinalizer(ctx, log, database); err != nil {
127-
log.Error(err, "Can't add finalizer to this custom-resource (database)")
88+
// Add finalizer for this CR if necessary
89+
if !controllerutil.ContainsFinalizer(database, databaseFinalizer) {
90+
controllerutil.AddFinalizer(database, databaseFinalizer)
91+
err = r.Update(ctx, database)
92+
if err != nil {
93+
return ctrl.Result{}, err
94+
}
95+
}
96+
97+
// Create database if necessary
98+
hasDatabase, err := db.HasDatabase(ctx, database.Spec.Database)
99+
if err != nil {
100+
log.Error(err, "Couldn't check if database '", database.Spec.Database, "' exists")
101+
return ctrl.Result{}, err
102+
} else if !hasDatabase {
103+
log.Info("Creating new database '", database.Spec.Database, "'")
104+
105+
err = db.CreateDatabase(ctx, database.Spec.Database)
106+
if err != nil {
107+
log.Error(err, "Can't create database")
108+
return ctrl.Result{}, err
109+
}
110+
111+
log.Info("Created database '", database.Spec.Database, "'")
112+
}
113+
114+
// Create database user with full access if necessary
115+
hasDatabaseUserWithAccess, err := db.HasDatabaseUserWithAccess(ctx, database.Spec.Database, database.Spec.Username)
116+
if err != nil {
117+
log.Error(err, "Can't check if user has access to database")
118+
return ctrl.Result{}, err
119+
} else if !hasDatabaseUserWithAccess {
120+
log.Info("Creating new user '" + database.Spec.Username + "' and granting access to database '" + database.Spec.Database + "'")
121+
122+
err = db.CreateDatabaseUser(ctx, database.Spec.Database, database.Spec.Username, database.Spec.Password)
123+
if err != nil {
124+
log.Error(err, "Can't create database user with access to database")
128125
return ctrl.Result{}, err
129126
}
127+
log.Info("Created user '" + database.Spec.Username + "' and granted access to database '" + database.Spec.Database + "'")
130128
}
131129

132130
return ctrl.Result{}, nil
@@ -168,73 +166,38 @@ func (r *DatabaseReconciler) getDatabaseConnection(ctx context.Context, database
168166
return nil, errors.NewBadRequest("Database type not supported")
169167
}
170168

171-
func (r *DatabaseReconciler) finalizeDatabase(ctx context.Context, log logr.Logger, database *anbratengithubiov1alpha1.Database) error {
172-
db, err := r.getDatabaseConnection(ctx, database.Spec.Type)
173-
if err != nil {
174-
return err
175-
}
176-
177-
defer db.Close(ctx)
178-
169+
func (r *DatabaseReconciler) finalizeDatabase(ctx context.Context, log logr.Logger, db adapters.DatabaseAdapter, database *anbratengithubiov1alpha1.Database) error {
170+
// remove database user and database access if it exists
179171
hasDatabaseUserWithAccess, err := db.HasDatabaseUserWithAccess(ctx, database.Spec.Database, database.Spec.Username)
180172
if err != nil {
181173
return err
182-
}
183-
184-
if hasDatabaseUserWithAccess {
185-
log.Info("Remove user '" + database.Spec.Username + "' and its access to the database '" + database.Spec.Database + "'")
174+
} else if hasDatabaseUserWithAccess {
175+
log.Info("Removing user '" + database.Spec.Username + "' and revoking access to the database '" + database.Spec.Database + "'")
186176

187177
err = db.DeleteDatabaseUser(ctx, database.Spec.Database, database.Spec.Username)
188178
if err != nil {
189179
return err
190180
}
181+
log.Info("Removed database user '" + database.Spec.Username + "' and revoked access to the database '" + database.Spec.Database + "'")
191182
}
192183

184+
// remove database if it exists
193185
hasDatabase, err := db.HasDatabase(ctx, database.Spec.Database)
194186
if err != nil {
195187
return err
196-
}
197-
198-
if hasDatabase {
199-
log.Info("Remove database: '" + database.Spec.Database + "'")
188+
} else if hasDatabase {
189+
log.Info("Removing database '" + database.Spec.Database + "'")
200190

201191
err = db.DeleteDatabase(ctx, database.Spec.Database)
202192
if err != nil {
203193
return err
204194
}
195+
log.Info("Removed database '" + database.Spec.Database + "'")
205196
}
206197

207-
err = db.DeleteDatabase(ctx, database.Spec.Database)
208-
if err != nil {
209-
return err
210-
}
211-
212-
log.Info("Database: '" + database.Spec.Database + "' and user: '" + database.Spec.Username + "' removed")
213-
return nil
214-
}
215-
216-
func (r *DatabaseReconciler) addFinalizer(ctx context.Context, log logr.Logger, m *anbratengithubiov1alpha1.Database) error {
217-
log.Info("Adding Finalizer for the database")
218-
controllerutil.AddFinalizer(m, databaseFinalizer)
219-
220-
// Update CR
221-
err := r.Update(ctx, m)
222-
if err != nil {
223-
log.Error(err, "Failed to update database with finalizer")
224-
return err
225-
}
226198
return nil
227199
}
228200

229-
func contains(list []string, s string) bool {
230-
for _, v := range list {
231-
if v == s {
232-
return true
233-
}
234-
}
235-
return false
236-
}
237-
238201
// SetupWithManager sets up the controller with the Manager.
239202
func (r *DatabaseReconciler) SetupWithManager(mgr ctrl.Manager) error {
240203
return ctrl.NewControllerManagedBy(mgr).

0 commit comments

Comments
 (0)