use mysql compatible configuration and allow duplicate keys in mysqlConf field of cr

Author: bing.ma

CHANGELOG.md

@@ -13,6 +13,8 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * Add SidecarImage fields to `.Spec` to allow specifying custom sidecar image.
 
 ### Changed
+* Change the type of `spec.mysqlConf` in the `mysqlclusters` crd to string
+
 ### Removed
 ### Fixed
 * Avoid set read_only conflict when graceful takeover

config/crd/bases/mysql.presslabs.org_mysqlclusters.yaml

@@ -98,13 +98,8 @@ spec:
                   description: The number of pods from that set that must still be available after the eviction, even in the absence of the evicted pod Defaults to 50%
                   type: string
                 mysqlConf:
-                  additionalProperties:
-                    anyOf:
-                      - type: integer
-                      - type: string
-                    x-kubernetes-int-or-string: true
-                  description: A map[string]string that will be passed to my.cnf file.
-                  type: object
+                  description: A string that will be passed to my.cnf file, it should be compatible with mysql config format.
+                  type: string
                 mysqlVersion:
                   description: 'Represents the MySQL version that will be run. The available version can be found here: https://github.com/bitpoke/mysql-operator/blob/0fd4641ce4f756a0aab9d31c8b1f1c44ee10fcb2/pkg/util/constants/constants.go#L87 This field should be set even if the Image is set to let the operator know which mysql version is running. Based on this version the operator can take decisions which features can be used. Defaults to 5.7'
                   type: string

deploy/charts/mysql-operator/crds/mysql.presslabs.org_mysqlclusters.yaml

@@ -99,13 +99,8 @@ spec:
                   description: The number of pods from that set that must still be available after the eviction, even in the absence of the evicted pod Defaults to 50%
                   type: string
                 mysqlConf:
-                  additionalProperties:
-                    anyOf:
-                      - type: integer
-                      - type: string
-                    x-kubernetes-int-or-string: true
-                  description: A map[string]string that will be passed to my.cnf file.
-                  type: object
+                  description: A string that will be passed to my.cnf file, it should be compatible with mysql config format.
+                  type: string
                 mysqlVersion:
                   description: 'Represents the MySQL version that will be run. The available version can be found here: https://github.com/bitpoke/mysql-operator/blob/0fd4641ce4f756a0aab9d31c8b1f1c44ee10fcb2/pkg/util/constants/constants.go#L87 This field should be set even if the Image is set to let the operator know which mysql version is running. Based on this version the operator can take decisions which features can be used. Defaults to 5.7'
                   type: string

examples/example-cluster.yaml

@@ -58,8 +58,9 @@ spec:
   # serverIDOffset: 100
 
   ## Configs that will be added to my.cnf for cluster
-  mysqlConf:
-  #   innodb-buffer-size: 128M
+  ## just write it the same way you write the MySQL configuration file
+  mysqlConf: |-
+  #   innodb-buffer-size = 128M
 
 
   ## Specify additional pod specification

pkg/apis/mysql/v1alpha1/mysqlcluster_defaults.go

@@ -50,7 +50,7 @@ func SetDefaults_MysqlCluster(c *MysqlCluster) {
 	}
 
 	if len(c.Spec.MysqlConf) == 0 {
-		c.Spec.MysqlConf = make(MysqlConf)
+		c.Spec.MysqlConf = ""
 	}
 
 	if len(c.Spec.MinAvailable) == 0 && *c.Spec.Replicas > 1 {

pkg/apis/mysql/v1alpha1/mysqlcluster_types.go

@@ -17,6 +17,9 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"fmt"
+	"strings"
+
 	core "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -100,7 +103,7 @@ type MysqlClusterSpec struct {
 	// +optional
 	BackupScheduleJobsHistoryLimit *int `json:"backupScheduleJobsHistoryLimit,omitempty"`
 
-	// A map[string]string that will be passed to my.cnf file.
+	// A string that will be passed to my.cnf file, it should be compatible with mysql config format.
 	// +optional
 	MysqlConf MysqlConf `json:"mysqlConf,omitempty"`
 
@@ -175,9 +178,100 @@ type MysqlClusterSpec struct {
 	InitFileExtraSQL []string `json:"initFileExtraSQL,omitempty"`
 }
 
-// MysqlConf defines type for extra cluster configs. It's a simple map between
-// string and string.
-type MysqlConf map[string]intstr.IntOrString
+// MysqlConf defines type for extra cluster configs. value is a mysql ini string, allow duplicate keys, like:
+// replicated_do_db=db1
+// replicated_do_db=db2
+type MysqlConf string
+
+// Get returns the value of key, if found, mysql allow no value in config, so we need to return a bool
+func (m MysqlConf) Get(key string) (string, bool) {
+	s := string(m)
+	lines := strings.Split(s, "\n")
+	for _, line := range lines {
+		if line == "" {
+			continue
+		}
+		keyAndValue := strings.Split(strings.TrimSpace(line), "=")
+		if strings.TrimSpace(keyAndValue[0]) == strings.TrimSpace(key) {
+			if len(keyAndValue) == 1 {
+				return "", true
+			}
+			if len(keyAndValue) == 2 {
+				return strings.TrimSpace(keyAndValue[1]), true
+			}
+		}
+	}
+	return "", false
+}
+
+// Set a key value pair, if key already exists, it will be overwritten else it will append at the end
+func (m MysqlConf) Set(key string, value string) MysqlConf {
+	s := string(m)
+	lines := strings.Split(s, "\n")
+	trimmedKey := strings.TrimSpace(key)
+	trimmedValue := strings.TrimSpace(value)
+	found := false
+	for index, line := range lines {
+		keyAndValue := strings.Split(strings.TrimSpace(line), "=")
+		if strings.TrimSpace(keyAndValue[0]) == trimmedKey {
+			found = true
+			// mysql allow bare boolean options with no value assignment,like:
+			// [mysqld]
+			// skip-name-resolve # no value here, when len(keyAndValue) == 1
+
+			// when len(keyAndValue) == 2 means "key = value"
+			if len(keyAndValue) == 1 && trimmedValue != "" || len(keyAndValue) == 2 {
+				// need to set key = value
+				lines[index] = fmt.Sprintf("%s = %s", trimmedKey, trimmedValue)
+			}
+		}
+	}
+	res := strings.Join(lines, "\n")
+	if !found {
+		if trimmedValue != "" {
+			res += fmt.Sprintf("\n%s = %s", trimmedKey, trimmedValue)
+		} else {
+			res += fmt.Sprintf("\n%s", trimmedKey)
+		}
+	}
+	return MysqlConf(res)
+}
+
+// ToMap returns a map[string]string, to compatible with addKVConfigsToSection
+func (m MysqlConf) ToMap() []map[string]intstr.IntOrString {
+	trimmed := strings.TrimSpace(string(m))
+	res := make([]map[string]intstr.IntOrString, 0)
+	if trimmed == "" {
+		return res
+	}
+	lines := strings.Split(trimmed, "\n")
+	total := make(map[string]intstr.IntOrString)
+	for _, line := range lines {
+		trimmedLine := strings.TrimSpace(line)
+		if trimmedLine == "" {
+			continue
+		}
+		keyAndValue := strings.Split(trimmedLine, "=")
+		length := len(keyAndValue)
+		if length != 1 && length != 2 {
+			continue
+		}
+		key := strings.TrimSpace(keyAndValue[0])
+		value := intstr.FromString("<nil>")
+		if length == 2 {
+			value = intstr.FromString(strings.TrimSpace(keyAndValue[1]))
+		}
+		if _, ok := total[key]; ok {
+			res = append(res, map[string]intstr.IntOrString{key: value})
+		} else {
+			total[key] = value
+		}
+	}
+	if len(total) != 0 {
+		res = append(res, total)
+	}
+	return res
+}
 
 // PodSpec defines type for configure cluster pod spec.
 type PodSpec struct {
@@ -376,7 +470,6 @@ type MysqlClusterStatus struct {
 // +kubebuilder:printcolumn:name="Replicas",type="integer",JSONPath=".spec.replicas",description="The number of desired nodes"
 // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
 // +kubebuilder:resource:shortName=mysql
-//
 type MysqlCluster struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -387,7 +480,6 @@ type MysqlCluster struct {
 
 // MysqlClusterList contains a list of MysqlCluster
 // +kubebuilder:object:root=true
-//
 type MysqlClusterList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata,omitempty"`

pkg/apis/mysql/v1alpha1/mysqlcluster_types_test.go

@@ -83,7 +83,7 @@ var _ = Describe("MysqlCluster CRUD", func() {
 		It("should populate fields defaults", func() {
 			SetDefaults_MysqlCluster(cluster)
 
-			Expect(cluster.Spec.MysqlConf).NotTo(BeNil())
+			Expect(cluster.Spec.MysqlConf).NotTo(Equal(""))
 		})
 	})
 

pkg/apis/mysql/v1alpha1/zz_generated.deepcopy.go

@@ -32,7 +32,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -170,9 +169,8 @@ var _ = Describe("MysqlBackupCron controller", func() {
 
 		It("should be just one entry for a cluster", func() {
 			// update cluster spec
-			cluster.Spec.MysqlConf = map[string]intstr.IntOrString{
-				"something": intstr.FromString("new"),
-			}
+			cluster.Spec.MysqlConf = "something = new"
+
 			Expect(c.Update(context.TODO(), cluster)).To(Succeed())
 
 			// expect an reconcile event

pkg/controller/mysqlbackupcron/mysqlbackupcron_controller_test.go

@@ -94,7 +94,10 @@ fi
 }
 
 func buildMysqlConfData(cluster *mysqlcluster.MysqlCluster) (string, error) {
-	cfg := ini.Empty()
+	// allow duplicate key in section
+	cfg := ini.Empty(ini.LoadOptions{
+		AllowShadows: true,
+	})
 	sec := cfg.Section("mysqld")
 
 	if cluster.GetMySQLSemVer().Major == 5 {
@@ -106,8 +109,13 @@ func buildMysqlConfData(cluster *mysqlcluster.MysqlCluster) (string, error) {
 	// boolean configs
 	addBConfigsToSection(sec, mysqlMasterSlaveBooleanConfigs)
 	// add custom configs, would overwrite common configs
-	addKVConfigsToSection(sec, convertMapToKVConfig(mysqlCommonConfigs), cluster.Spec.MysqlConf)
-
+	extraMysqld := cluster.Spec.MysqlConf.ToMap()
+	if extraMysqld != nil {
+		extraMysqld = append(extraMysqld, convertMapToKVConfig(mysqlCommonConfigs))
+	} else {
+		extraMysqld = make([]map[string]intstr.IntOrString, 0)
+	}
+	addKVConfigsToSection(sec, extraMysqld...)
 	// include configs from /etc/mysql/conf.d/*.cnf
 	_, err := sec.NewBooleanKey(fmt.Sprintf("!includedir %s", ConfDPath))
 	if err != nil {
@@ -120,7 +128,6 @@ func buildMysqlConfData(cluster *mysqlcluster.MysqlCluster) (string, error) {
 	}
 
 	return data, nil
-
 }
 
 func convertMapToKVConfig(m map[string]string) map[string]intstr.IntOrString {
@@ -146,8 +153,15 @@ func addKVConfigsToSection(s *ini.Section, extraMysqld ...map[string]intstr.IntO
 
 		for _, k := range keys {
 			value := extra[k]
-			if _, err := s.NewKey(k, value.String()); err != nil {
-				log.Error(err, "failed to add key to config section", "key", k, "value", extra[k], "section", s)
+			// in (m MysqlConf) ToMap() we set it to "<nil>" when spec.mysqlConf no value.
+			if value.String() == "<nil>" {
+				if _, err := s.NewBooleanKey(k); err != nil {
+					log.Error(err, "failed to add boolean key to config section", "key", k)
+				}
+			} else {
+				if _, err := s.NewKey(k, value.String()); err != nil {
+					log.Error(err, "failed to add key to config section", "key", k, "value", extra[k], "section", s)
+				}
 			}
 		}
 	}