Skip to content

Commit ab25c5d

Browse files
authored
fix: memory and mongodb connection leak (#45)
Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
1 parent adb870c commit ab25c5d

File tree

4 files changed

+29
-10
lines changed

4 files changed

+29
-10
lines changed

internal/controllers/instance_controller.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ const (
6666
)
6767

6868
// MongoDBProvider returns a storage.Database for MongoDB
69-
func MongoDBProvider(ctx context.Context, instance v1beta1.GrowthbookInstance, username, password string) (storage.Database, error) {
69+
func MongoDBProvider(ctx context.Context, instance v1beta1.GrowthbookInstance, username, password string) (storage.Disconnector, storage.Database, error) {
7070
opts := options.Client().ApplyURI(instance.Spec.MongoDB.URI)
7171
if username != "" || password != "" {
7272
opts.SetAuth(options.Credential{
@@ -78,22 +78,22 @@ func MongoDBProvider(ctx context.Context, instance v1beta1.GrowthbookInstance, u
7878
opts.SetAppName("k8sgrowthbook-controller")
7979
mongoClient, err := mongo.NewClient(opts)
8080
if err != nil {
81-
return nil, err
81+
return nil, nil, err
8282
}
8383

8484
u, err := url.Parse(instance.Spec.MongoDB.URI)
8585
if err != nil {
86-
return nil, err
86+
return nil, nil, err
8787
}
8888

8989
dbName := strings.TrimLeft(u.Path, "/")
9090
db := mongoClient.Database(dbName)
9191

9292
if err := mongoClient.Connect(ctx); err != nil {
93-
return nil, fmt.Errorf("failed connecting to mongodb: %w", err)
93+
return nil, nil, fmt.Errorf("failed connecting to mongodb: %w", err)
9494
}
9595

96-
return mongodb.New(db), nil
96+
return mongoClient, mongodb.New(db), nil
9797
}
9898

9999
// GrowthbookInstance reconciles a GrowthbookInstance object
@@ -102,7 +102,7 @@ type GrowthbookInstanceReconciler struct {
102102
Log logr.Logger
103103
Scheme *runtime.Scheme
104104
Recorder record.EventRecorder
105-
DatabaseProvider func(ctx context.Context, instance v1beta1.GrowthbookInstance, username, password string) (storage.Database, error)
105+
DatabaseProvider func(ctx context.Context, instance v1beta1.GrowthbookInstance, username, password string) (storage.Disconnector, storage.Database, error)
106106
}
107107

108108
type GrowthbookInstanceReconcilerOptions struct {
@@ -332,11 +332,19 @@ func (r *GrowthbookInstanceReconciler) reconcile(ctx context.Context, instance v
332332
}
333333
}
334334

335-
db, err := r.DatabaseProvider(ctx, instance, usr, pw)
335+
disconnector, db, err := r.DatabaseProvider(ctx, instance, usr, pw)
336336
if err != nil {
337337
return instance, err
338338
}
339339

340+
defer func() {
341+
ctx, cancel := context.WithTimeout(context.TODO(), time.Second*10)
342+
defer cancel()
343+
if err := disconnector.Disconnect(ctx); err != nil {
344+
logger.Error(err, "failed disconnet mongodb")
345+
}
346+
}()
347+
340348
instance.Status.SubResourceCatalog = []v1beta1.ResourceReference{}
341349

342350
instance, err = r.reconcileUsers(ctx, instance, db, logger)

internal/controllers/instance_controller_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,16 @@ import (
1616
)
1717

1818
// MockProvider returns a storage.Database for testing
19-
func MockProvider(ctx context.Context, instance v1beta1.GrowthbookInstance, username, password string) (storage.Database, error) {
19+
func MockProvider(ctx context.Context, instance v1beta1.GrowthbookInstance, username, password string) (storage.Disconnector, storage.Database, error) {
2020
// See test "reconciling a GrowthbookInstance with a timeout"
2121
if instance.Spec.Timeout != nil && instance.Spec.Timeout.Duration == 500*time.Millisecond {
2222
<-ctx.Done()
23-
return nil, ctx.Err()
23+
return nil, nil, ctx.Err()
2424
}
2525

2626
// For testing the controller logic the storage adapter just does nothing and returns no error
2727
if instance.Spec.MongoDB.URI == "" {
28-
return &growthbook.MockDatabase{
28+
return &growthbook.MockDisconnect{}, &growthbook.MockDatabase{
2929
FindOne: func(ctx context.Context, filter interface{}, dst interface{}) error {
3030
return nil
3131
},

internal/growthbook/storage_mock.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ var (
1212
_ storage.Collection = &MockCollection{}
1313
)
1414

15+
type MockDisconnect struct {
16+
}
17+
18+
func (d *MockDisconnect) Disconnect(ctx context.Context) error {
19+
return nil
20+
}
21+
1522
type MockDatabase struct {
1623
FindOne func(ctx context.Context, filter interface{}, dst interface{}) error
1724
DeleteOne func(ctx context.Context, filter interface{}) error

internal/storage/storage.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ package storage
22

33
import "context"
44

5+
type Disconnector interface {
6+
Disconnect(ctx context.Context) error
7+
}
8+
59
type Database interface {
610
Collection(collName string) Collection
711
}

0 commit comments

Comments
 (0)