Merge pull request radondb#472 from acekingke/FixMemLeak

*: Fix memory Leak of Operator. radondb#469
zhl003 · May 24, 2022 · aea788d · aea788d
2 parents 1208e36 + e439ea1
commit aea788d
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 66 deletions.
diff --git a/backup/backup.go b/backup/backup.go
@@ -19,23 +19,24 @@ package backup
 import (
 	"fmt"
 
+	"github.com/go-logr/logr"
 	"github.com/radondb/radondb-mysql-kubernetes/utils"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	v1alhpa1 "github.com/radondb/radondb-mysql-kubernetes/api/v1alpha1"
 )
 
-var log = logf.Log.WithName("backup")
-
 // Backup is a type wrapper over Backup that contains the Business logic
 type Backup struct {
 	*v1alhpa1.Backup
+	Log logr.Logger
 }
 
 // New returns a wraper object over Backup
 func New(backup *v1alhpa1.Backup) *Backup {
 	return &Backup{
 		Backup: backup,
+		Log:    logf.Log.WithName("backup"),
 	}
 }
 

diff --git a/backup/status.go b/backup/status.go
@@ -40,26 +40,26 @@ func (c *Backup) UpdateStatusCondition(condType apiv1alpha1.BackupConditionType,
 	t := time.Now()
 
 	if len(c.Status.Conditions) == 0 {
-		log.V(4).Info(fmt.Sprintf("Setting lastTransitionTime for mysql backup "+
+		c.Log.V(4).Info(fmt.Sprintf("Setting lastTransitionTime for mysql backup "+
 			"%q condition %q to %v", c.Name, condType, t))
 		newCondition.LastTransitionTime = metav1.NewTime(t)
 		c.Status.Conditions = []apiv1alpha1.BackupCondition{newCondition}
 	} else {
 		if i, exist := c.condExists(condType); exist {
 			cond := c.Status.Conditions[i]
 			if cond.Status != newCondition.Status {
-				log.V(3).Info(fmt.Sprintf("Found status change for mysql backup "+
+				c.Log.V(3).Info(fmt.Sprintf("Found status change for mysql backup "+
 					"%q condition %q: %q -> %q; setting lastTransitionTime to %v",
 					c.Name, condType, cond.Status, status, t))
 				newCondition.LastTransitionTime = metav1.NewTime(t)
 			} else {
 				newCondition.LastTransitionTime = cond.LastTransitionTime
 			}
-			log.V(4).Info(fmt.Sprintf("Setting lastTransitionTime for mysql backup "+
+			c.Log.V(4).Info(fmt.Sprintf("Setting lastTransitionTime for mysql backup "+
 				"%q condition %q to %q", c.Name, condType, status))
 			c.Status.Conditions[i] = newCondition
 		} else {
-			log.V(4).Info(fmt.Sprintf("Setting new condition for mysql backup %q, condition %q to %q",
+			c.Log.V(4).Info(fmt.Sprintf("Setting new condition for mysql backup %q, condition %q to %q",
 				c.Name, condType, status))
 			newCondition.LastTransitionTime = metav1.NewTime(t)
 			c.Status.Conditions = append(c.Status.Conditions, newCondition)

diff --git a/backup/syncer/job.go b/backup/syncer/job.go
@@ -25,16 +25,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	v1alpha1 "github.com/radondb/radondb-mysql-kubernetes/api/v1alpha1"
 	"github.com/radondb/radondb-mysql-kubernetes/backup"
 	"github.com/radondb/radondb-mysql-kubernetes/mysqlcluster"
 	"github.com/radondb/radondb-mysql-kubernetes/utils"
 )
 
-var log = logf.Log.WithName("backup.syncer.job")
-
 type jobSyncer struct {
 	job    *batchv1.Job
 	backup *backup.Backup
@@ -59,7 +56,7 @@ func NewJobSyncer(c client.Client, s *runtime.Scheme, backup *backup.Backup) syn
 
 func (s *jobSyncer) SyncFn() error {
 	if s.backup.Status.Completed {
-		log.V(1).Info("backup already completed", "backup", s.backup)
+		s.backup.Log.V(1).Info("backup already completed", "backup", s.backup)
 		// skip doing anything
 		return syncer.ErrIgnore
 	}

diff --git a/mysqlcluster/mysqlcluster.go b/mysqlcluster/mysqlcluster.go
@@ -35,6 +35,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
+	"github.com/go-logr/logr"
 	apiv1alpha1 "github.com/radondb/radondb-mysql-kubernetes/api/v1alpha1"
 	"github.com/radondb/radondb-mysql-kubernetes/utils"
 )
@@ -47,17 +48,17 @@ const (
 	gb
 )
 
-var log = logf.Log.WithName("mysqlcluster")
-
 // MysqlCluster is the wrapper for apiv1alpha1.MysqlCluster type.
 type MysqlCluster struct {
 	*apiv1alpha1.MysqlCluster
+	log logr.Logger
 }
 
 // New returns a pointer to MysqlCluster.
 func New(mc *apiv1alpha1.MysqlCluster) *MysqlCluster {
 	return &MysqlCluster{
 		MysqlCluster: mc,
+		log:          logf.Log.WithName("mysqlcluster"),
 	}
 }
 
@@ -73,7 +74,7 @@ func (c *MysqlCluster) Validate() error {
 	// MySQL8 nerver support TokuDB
 	// https://www.percona.com/blog/2021/05/21/tokudb-support-changes-and-future-removal-from-percona-server-for-mysql-8-0/
 	if c.Spec.MysqlVersion == "8.0" && c.Spec.MysqlOpts.InitTokuDB {
-		log.Info("TokuDB is not supported in MySQL 8.0 any more, the value in Cluster.spec.mysqlOpts.initTokuDB should be set false")
+		c.log.Info("TokuDB is not supported in MySQL 8.0 any more, the value in Cluster.spec.mysqlOpts.initTokuDB should be set false")
 		return nil
 	}
 	// https://github.com/percona/percona-docker/blob/main/percona-server-5.7/ps-entry.sh#L159
@@ -135,7 +136,7 @@ func (c *MysqlCluster) GetMySQLVersion() string {
 		version = v
 	} else {
 		errmsg := "Invalid mysql version option:" + c.Spec.MysqlVersion
-		log.Error(errors.New(errmsg), "currently we do not support mysql 5.6 or earlier version, default mysql version option should be 5.7 or 8.0")
+		c.log.Error(errors.New(errmsg), "currently we do not support mysql 5.6 or earlier version, default mysql version option should be 5.7 or 8.0")
 		return utils.InvalidMySQLVersion
 	}
 

diff --git a/mysqlcluster/mysqlcluster_test.go b/mysqlcluster/mysqlcluster_test.go
@@ -31,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	mysqlv1alpha1 "github.com/radondb/radondb-mysql-kubernetes/api/v1alpha1"
 	"github.com/radondb/radondb-mysql-kubernetes/utils"
@@ -46,13 +47,13 @@ var (
 		},
 	}
 	testCluster = MysqlCluster{
-		&mysqlCluster,
+		&mysqlCluster, logf.Log.WithName("mysqlcluster"),
 	}
 )
 
 func TestNew(t *testing.T) {
 	want := &MysqlCluster{
-		&mysqlCluster,
+		&mysqlCluster, logf.Log.WithName("mysqlcluster"),
 	}
 	assert.Equal(t, want, New(&mysqlCluster))
 }
@@ -69,7 +70,8 @@ func TestGetLabel(t *testing.T) {
 			"app.kubernetes.io/instance": "instance",
 		}
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster,
+			log:          logf.Log.WithName("mysqlcluster"),
 		}
 		want := labels.Set{
 			"mysql.radondb.com/cluster":  "sample",
@@ -88,7 +90,8 @@ func TestGetLabel(t *testing.T) {
 			"app.kubernetes.io/component": "component",
 		}
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster,
+			log:          logf.Log.WithName("mysqlcluster"),
 		}
 		want := labels.Set{
 			"mysql.radondb.com/cluster":  "sample",
@@ -107,7 +110,8 @@ func TestGetLabel(t *testing.T) {
 			"app.kubernetes.io/part-of": "part-of",
 		}
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster,
+			log:          logf.Log.WithName("mysqlcluster"),
 		}
 		want := labels.Set{
 			"mysql.radondb.com/cluster":  "sample",
@@ -137,7 +141,8 @@ func TestGetMySQLVersion(t *testing.T) {
 		testMysqlCluster := mysqlCluster
 		testMysqlCluster.Spec.MysqlVersion = "8.0"
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster,
+			log:          logf.Log.WithName("mysqlcluster"),
 		}
 		want := "8.0.25"
 		assert.Equal(t, want, testCase.GetMySQLVersion())
@@ -147,7 +152,8 @@ func TestGetMySQLVersion(t *testing.T) {
 		testMysqlCluster := mysqlCluster
 		testMysqlCluster.Spec.MysqlVersion = "5.7"
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster,
+			log:          logf.Log.WithName("mysqlcluster"),
 		}
 		want := "5.7.34"
 		assert.Equal(t, want, testCase.GetMySQLVersion())
@@ -157,7 +163,8 @@ func TestGetMySQLVersion(t *testing.T) {
 		testMysqlCluster := mysqlCluster
 		testMysqlCluster.Spec.MysqlVersion = "5.7.34"
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster,
+			log:          logf.Log.WithName("mysqlcluster"),
 		}
 		want := utils.InvalidMySQLVersion
 		assert.Equal(t, want, testCase.GetMySQLVersion())
@@ -172,7 +179,8 @@ func TestCreatePeers(t *testing.T) {
 		testMysqlCluster.ObjectMeta.Namespace = "default"
 		testMysqlCluster.Spec.Replicas = &replicas
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster,
+			log:          logf.Log.WithName("mysqlcluster"),
 		}
 		want := "sample-mysql-0.sample-mysql.default:8801,sample-mysql-1.sample-mysql.default:8801"
 		assert.Equal(t, want, testCase.CreatePeers())
@@ -183,7 +191,7 @@ func TestCreatePeers(t *testing.T) {
 		testMysqlCluster.ObjectMeta.Namespace = "default"
 		testMysqlCluster.Spec.Replicas = &replicas
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster, log: logf.Log.WithName("mysqlcluster"),
 		}
 		want := "sample-mysql-0.sample-mysql.default:8801,sample-mysql-1.sample-mysql.default:8801,sample-mysql-2.sample-mysql.default:8801"
 		assert.Equal(t, want, testCase.CreatePeers())
@@ -194,7 +202,7 @@ func TestCreatePeers(t *testing.T) {
 		testMysqlCluster.ObjectMeta.Namespace = "default"
 		testMysqlCluster.Spec.Replicas = &replicas
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster, log: logf.Log.WithName("mysqlcluster"),
 		}
 		want := ""
 		for i := 0; i < 666; i++ {
@@ -212,7 +220,7 @@ func TestCreatePeers(t *testing.T) {
 		testMysqlCluster.ObjectMeta.Namespace = "default"
 		testMysqlCluster.Spec.Replicas = &replicas
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster, log: logf.Log.WithName("mysqlcluster"),
 		}
 		want := ""
 		assert.Equal(t, want, testCase.CreatePeers())
@@ -223,7 +231,7 @@ func TestCreatePeers(t *testing.T) {
 		testMysqlCluster.ObjectMeta.Namespace = "default"
 		testMysqlCluster.Spec.Replicas = &replicas
 		testCase := MysqlCluster{
-			&testMysqlCluster,
+			MysqlCluster: &testMysqlCluster, log: logf.Log.WithName("mysqlcluster"),
 		}
 		want := ""
 		assert.Equal(t, want, testCase.CreatePeers())
@@ -234,7 +242,7 @@ func TestGetPodHostName(t *testing.T) {
 	testMysqlCluster := mysqlCluster
 	testMysqlCluster.ObjectMeta.Namespace = "default"
 	testCase := MysqlCluster{
-		&testMysqlCluster,
+		MysqlCluster: &testMysqlCluster, log: logf.Log.WithName("mysqlcluster"),
 	}
 	want0 := "sample-mysql-0.sample-mysql.default"
 	want1 := "sample-mysql-1.sample-mysql.default"
@@ -314,7 +322,7 @@ func TestEnsureVolumes(t *testing.T) {
 		testMysql := mysqlCluster
 		testMysql.Spec.Persistence.Enabled = false
 		testCase := MysqlCluster{
-			&testMysql,
+			MysqlCluster: &testMysql, log: logf.Log.WithName("mysqlcluster"),
 		}
 		want := []corev1.Volume{
 			{
@@ -333,7 +341,7 @@ func TestEnsureVolumes(t *testing.T) {
 		testMysql.Spec.Persistence.Enabled = true
 		testMysql.Spec.MysqlOpts.InitTokuDB = true
 		testCase := MysqlCluster{
-			&testMysql,
+			MysqlCluster: &testMysql, log: logf.Log.WithName("mysqlcluster"),
 		}
 		want := []corev1.Volume{
 			{
@@ -353,7 +361,7 @@ func TestEnsureVolumes(t *testing.T) {
 		testMysql := mysqlCluster
 		testMysql.Spec.Persistence.Enabled = true
 		testCase := MysqlCluster{
-			&testMysql,
+			MysqlCluster: &testMysql, log: logf.Log.WithName("mysqlcluster"),
 		}
 		assert.Equal(t, volume, testCase.EnsureVolumes())
 	}
@@ -388,7 +396,7 @@ func TestEnsureVolumeClaimTemplates(t *testing.T) {
 			},
 		}
 		testCase := MysqlCluster{
-			&testMysql,
+			&testMysql, logf.Log.WithName("mysqlcluster"),
 		}
 		want := []corev1.PersistentVolumeClaim{
 			{
@@ -433,7 +441,7 @@ func TestEnsureVolumeClaimTemplates(t *testing.T) {
 		testMysql.Spec.Persistence.Size = "10Gi"
 		testMysql.Spec.Persistence.StorageClass = &storageClass
 		testCase := MysqlCluster{
-			&testMysql,
+			&testMysql, logf.Log.WithName("mysqlcluster"),
 		}
 		guard := gomonkey.ApplyFunc(controllerutil.SetControllerReference, func(_ metav1.Object, _ metav1.Object, _ *runtime.Scheme) error {
 			return nil
@@ -451,7 +459,7 @@ func TestEnsureVolumeClaimTemplates(t *testing.T) {
 		testMysql.Spec.Persistence.Enabled = true
 		testMysql.Spec.Persistence.Size = "10Gi"
 		testCase := MysqlCluster{
-			&testMysql,
+			&testMysql, logf.Log.WithName("mysqlcluster"),
 		}
 		guard := gomonkey.ApplyFunc(controllerutil.SetControllerReference, func(_ metav1.Object, _ metav1.Object, _ *runtime.Scheme) error {
 			return fmt.Errorf("test")
@@ -521,7 +529,7 @@ func TestEnsureMysqlConf(t *testing.T) {
 	{
 		testMysqlCase := testMysql
 		testCase := MysqlCluster{
-			&testMysqlCase,
+			&testMysqlCase, logf.Log.WithName("mysqlcluster"),
 		}
 		testCase.EnsureMysqlConf()
 		wantSize = strconv.FormatUint(uint64(0.45*float64(gb)), 10)
@@ -539,7 +547,7 @@ func TestEnsureMysqlConf(t *testing.T) {
 		testMysqlCase := testMysql
 		testMysqlCase.Spec.MysqlOpts.MysqlConf["innodb_buffer_pool_size"] = strconv.FormatUint(uint64(600*mb), 10)
 		testCase := MysqlCluster{
-			&testMysqlCase,
+			&testMysqlCase, logf.Log.WithName("mysqlcluster"),
 		}
 		testCase.EnsureMysqlConf()
 		wantSize := strconv.FormatUint(uint64(600*float64(mb)), 10)
@@ -559,7 +567,7 @@ func TestEnsureMysqlConf(t *testing.T) {
 		testMysqlCase.Spec.MysqlOpts.Resources.Requests["memory"] = *memoryCase
 		testMysqlCase.Spec.MysqlOpts.MysqlConf["innodb_buffer_pool_size"] = strconv.FormatUint(uint64(1.7*float64(gb)), 10)
 		testCase := MysqlCluster{
-			&testMysqlCase,
+			&testMysqlCase, logf.Log.WithName("mysqlcluster"),
 		}
 		testCase.EnsureMysqlConf()
 		wantSize := strconv.FormatUint(uint64(1.6*float64(gb)), 10)
@@ -578,7 +586,7 @@ func TestEnsureMysqlConf(t *testing.T) {
 		testMysqlCase.Spec.MysqlOpts.Resources.Requests["memory"] = *memoryCase
 		testMysqlCase.Spec.MysqlOpts.MysqlConf["innodb_buffer_pool_size"] = strconv.FormatUint(uint64(1.7*float64(gb)), 10)
 		testCase := MysqlCluster{
-			&testMysqlCase,
+			&testMysqlCase, logf.Log.WithName("mysqlcluster"),
 		}
 		testCase.EnsureMysqlConf()
 		wantSize := strconv.FormatUint(uint64(1.2*float64(gb)), 10)
@@ -599,7 +607,7 @@ func TestEnsureMysqlConf(t *testing.T) {
 		testMysqlCase.Spec.MysqlOpts.Resources.Limits["cpu"] = *limitCpucorev1sCase
 		testMysqlCase.Spec.MysqlOpts.Resources.Requests["memory"] = *memoryCase
 		testCase := MysqlCluster{
-			&testMysqlCase,
+			&testMysqlCase, logf.Log.WithName("mysqlcluster"),
 		}
 		testCase.EnsureMysqlConf()
 		wantSize := strconv.FormatUint(uint64(2*float64(gb)), 10)

diff --git a/mysqlcluster/syncer/mysql_cm.go b/mysqlcluster/syncer/mysql_cm.go
@@ -23,9 +23,11 @@ import (
 
 	"github.com/go-ini/ini"
 	"github.com/presslabs/controller-util/syncer"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/radondb/radondb-mysql-kubernetes/mysqlcluster"
 	"github.com/radondb/radondb-mysql-kubernetes/utils"
@@ -66,6 +68,7 @@ func NewMysqlCMSyncer(cli client.Client, c *mysqlcluster.MysqlCluster) syncer.In
 
 // buildMysqlConf build the mysql config.
 func buildMysqlConf(c *mysqlcluster.MysqlCluster) (string, error) {
+	var log = logf.Log.WithName("mysqlcluster.syncer.buildMysqlConf")
 	cfg := ini.Empty(ini.LoadOptions{IgnoreInlineComment: true})
 	sec := cfg.Section("mysqld")
 
@@ -114,6 +117,7 @@ func buildMysqlPluginConf(c *mysqlcluster.MysqlCluster) (string, error) {
 
 // addKVConfigsToSection add a map[string]string to a ini.Section
 func addKVConfigsToSection(s *ini.Section, extraMysqld ...map[string]string) {
+	var log = logf.Log.WithName("mysqlcluster.syncer.addKVConfigsToSection")
 	for _, extra := range extraMysqld {
 		keys := []string{}
 		for key := range extra {