K8SPSMDB-1504 Fix MongoDB Connection Leaks in PBM Operations (#2098)

* fix con

* fix lint and add test

* log pbm connection closing error & improve error message in e2e test

* uniform error messages for e2e test

---------

Co-authored-by: George Kechagias <[email protected]>

Author: 2 people

e2e-tests/init-deploy/compare/statefulset_another-name-rs0.yml

@@ -161,6 +161,57 @@ spec:
           resources: {}
           terminationMessagePath: /dev/termination-log
           terminationMessagePolicy: File
+        - args:
+            - pbm-agent-entrypoint
+          command:
+            - /opt/percona/pbm-entry.sh
+          env:
+            - name: PBM_AGENT_MONGODB_USERNAME
+              valueFrom:
+                secretKeyRef:
+                  key: MONGODB_BACKUP_USER_ESCAPED
+                  name: internal-another-name-users
+                  optional: false
+            - name: PBM_AGENT_MONGODB_PASSWORD
+              valueFrom:
+                secretKeyRef:
+                  key: MONGODB_BACKUP_PASSWORD_ESCAPED
+                  name: internal-another-name-users
+                  optional: false
+            - name: PBM_MONGODB_REPLSET
+              value: rs0
+            - name: PBM_MONGODB_PORT
+              value: "27017"
+            - name: PBM_AGENT_SIDECAR
+              value: "true"
+            - name: PBM_AGENT_SIDECAR_SLEEP
+              value: "5"
+            - name: POD_NAME
+              valueFrom:
+                fieldRef:
+                  apiVersion: v1
+                  fieldPath: metadata.name
+            - name: PBM_MONGODB_URI
+              value: mongodb://$(PBM_AGENT_MONGODB_USERNAME):$(PBM_AGENT_MONGODB_PASSWORD)@localhost:$(PBM_MONGODB_PORT)
+            - name: PBM_AGENT_TLS_ENABLED
+              value: "false"
+          imagePullPolicy: Always
+          name: backup-agent
+          resources: {}
+          securityContext:
+            runAsNonRoot: true
+            runAsUser: 1001
+          terminationMessagePath: /dev/termination-log
+          terminationMessagePolicy: File
+          volumeMounts:
+            - mountPath: /etc/mongodb-ssl
+              name: ssl
+              readOnly: true
+            - mountPath: /opt/percona
+              name: bin
+              readOnly: true
+            - mountPath: /data/db
+              name: mongod-data
         - args:
             - fluent-bit
           command:

e2e-tests/init-deploy/conf/another-name-rs0.yml

@@ -11,10 +11,18 @@ spec:
   tls:
     mode: disabled
   backup:
-    enabled: false
-    image: perconalab/percona-server-mongodb-operator:0.4.0-backup
-#    storages:
-#    tasks:
+    enabled: true
+    image: perconalab/percona-server-mongodb-operator:1.1.0-backup
+    storages:
+      gcp-cs-s3:
+        type: s3
+        s3:
+          credentialsSecret: gcp-cs-secret
+          region: us-east-1
+          bucket: operator-testing
+          prefix: psmdb-demand-backup-physical
+          endpointUrl: https://storage.googleapis.com
+          insecureSkipTLSVerify: false
   replsets:
   - name: rs0
     terminationGracePeriodSeconds: 300
@@ -70,4 +78,4 @@ spec:
     users: some-users
   logcollector:
     enabled: true
-    image: perconalab/fluentbit:main-logcollector
+    image: perconalab/fluentbit:main-logcollector

e2e-tests/init-deploy/run

@@ -76,6 +76,7 @@ compare_mongo_cmd "find" "myApp:myPass@$cluster-2.$cluster.$namespace"
 desc 'check number of connections'
 conn_count=$(run_mongo 'db.serverStatus().connections.current' "clusterAdmin:clusterAdmin123456@$cluster.$namespace" | egrep -v 'I NETWORK|W NETWORK|Error saving history file|Percona Server for MongoDB|connecting to:|Unable to reach primary for set|Implicit session:|versions do not match|bye')
 if [ ${conn_count} -gt ${max_conn} ]; then
+	echo "error: connection count (${conn_count}) exceeds maximum allowed (${max_conn})"
 	exit 1
 fi
 
@@ -94,6 +95,7 @@ compare_mongo_cmd "find" "myApp:myPass@$cluster-2.$cluster.$namespace" "-2nd"
 
 desc 'check if possible to create second cluster'
 cluster2="another-name-rs0"
+apply_s3_storage_secrets
 apply_cluster $test_dir/conf/$cluster2.yml
 desc 'check if all 3 Pods started'
 wait_for_running $cluster2 3
@@ -112,6 +114,15 @@ compare_mongo_cmd "find" "myApp:myPass@$cluster2-0.$cluster2.$namespace" "-3rd"
 compare_mongo_cmd "find" "myApp:myPass@$cluster2-1.$cluster2.$namespace" "-3rd"
 compare_mongo_cmd "find" "myApp:myPass@$cluster2-2.$cluster2.$namespace" "-3rd"
 
+desc 'check number of connections with backup'
+max_conn=50
+sleep 300
+conn_count=$(run_mongo 'db.serverStatus().connections.current' "clusterAdmin:clusterAdmin123456@$cluster2.$namespace" | egrep -v 'I NETWORK|W NETWORK|Error saving history file|Percona Server for MongoDB|connecting to:|Unable to reach primary for set|Implicit session:|versions do not match|bye')
+if [ ${conn_count} -gt ${max_conn} ]; then
+	echo "error: connection count (${conn_count}) exceeds maximum allowed (${max_conn}) with backup enabled"
+	exit 1
+fi
+
 desc 'check if mongod log files exist in pod'
 log_files=$(kubectl exec "${cluster2}-0" -c "mongod" -- ls "/data/db/logs" 2>/dev/null)
 if ! echo "$log_files" | grep -q 'mongod.log' || ! echo "$log_files" | grep -q 'mongod.full.log'; then

pkg/controller/perconaservermongodb/pbm.go

@@ -122,6 +122,11 @@ func (r *ReconcilePerconaServerMongoDB) reconcilePBMConfig(ctx context.Context,
 	if err != nil {
 		return errors.Wrap(err, "new PBM connection")
 	}
+	defer func() {
+		if err := pbm.Close(ctx); err != nil {
+			log.Error(err, "failed to close PBM connection")
+		}
+	}()
 
 	currentCfg, err := pbm.GetConfig(ctx)
 	if err != nil && !backup.IsErrNoDocuments(err) {
@@ -324,7 +329,11 @@ func (r *ReconcilePerconaServerMongoDB) reconcilePiTRConfig(ctx context.Context,
 	if err != nil {
 		return errors.Wrap(err, "create pbm object")
 	}
-	defer pbm.Close(ctx)
+	defer func() {
+		if err := pbm.Close(ctx); err != nil {
+			log.Error(err, "failed to close PBM connection")
+		}
+	}()
 
 	if err := enablePiTRIfNeeded(ctx, pbm, cr); err != nil {
 		if backup.IsErrNoDocuments(err) {
@@ -594,6 +603,11 @@ func (r *ReconcilePerconaServerMongoDB) resyncPBMIfNeeded(ctx context.Context, c
 			log.Error(err, "failed to open PBM connection")
 			return
 		}
+		defer func() {
+			if err := pbm.Close(ctx); err != nil {
+				log.Error(err, "failed to close PBM connection")
+			}
+		}()
 
 		log.Info("starting resync for main storage")
 

pkg/controller/perconaservermongodbrestore/logical.go

@@ -36,7 +36,11 @@ func (r *ReconcilePerconaServerMongoDBRestore) reconcileLogicalRestore(
 		status.State = psmdbv1.RestoreStateWaiting
 		return status, nil
 	}
-	defer pbmc.Close(ctx)
+	defer func() {
+		if err := pbmc.Close(ctx); err != nil {
+			log.Error(err, "failed to close PBM connection")
+		}
+	}()
 
 	if status.State == psmdbv1.RestoreStateNew || status.State == psmdbv1.RestoreStateWaiting {
 		// Disable PITR before restore

pkg/controller/perconaservermongodbrestore/physical.go

@@ -800,6 +800,8 @@ func (r *ReconcilePerconaServerMongoDBRestore) updatePBMConfigSecret(
 	cluster *psmdbv1.PerconaServerMongoDB,
 	bcp *psmdbv1.PerconaServerMongoDBBackup,
 ) error {
+	log := logf.FromContext(ctx)
+
 	secret := corev1.Secret{}
 	err := r.client.Get(ctx, types.NamespacedName{Name: r.pbmConfigName(cluster), Namespace: cluster.Namespace}, &secret)
 	if client.IgnoreNotFound(err) != nil {
@@ -810,6 +812,11 @@ func (r *ReconcilePerconaServerMongoDBRestore) updatePBMConfigSecret(
 	if err != nil {
 		return errors.Wrap(err, "new PBM connection")
 	}
+	defer func() {
+		if err := pbmC.Close(ctx); err != nil {
+			log.Error(err, "failed to close PBM connection")
+		}
+	}()
 
 	// PBM uses main storage to store restore metadata
 	// regardless of backup storage. See PBM-1503.

pkg/controller/perconaservermongodbrestore/psmdb_restore_controller.go

@@ -406,6 +406,11 @@ func (r *ReconcilePerconaServerMongoDBRestore) resyncStorage(
 	if err != nil {
 		return errors.Wrap(err, "new PBM connection")
 	}
+	defer func() {
+		if err := pbmC.Close(ctx); err != nil {
+			log.Error(err, "failed to close PBM connection")
+		}
+	}()
 
 	// restore: backupSource
 	if len(cr.Spec.BackupName) == 0 {