fix(tidb): support keyspace in config file if spec.keyspace is unset and remove unnecessary validation (#6451)

Signed-off-by: liubo02 <[email protected]>

Author: liubog2008

api/core/v1alpha1/tidb_types.go

@@ -325,8 +325,6 @@ type TiDBSpec struct {
 
 	// TiDBTemplateSpec embedded some fields managed by TiDBGroup.
 	// +kubebuilder:validation:XValidation:rule="!has(self.mode) || self.mode == 'Normal' || !has(self.keyspace) || self.keyspace.size() == 0",message="keyspace cannot be set if mode is StandBy"
-	// +kubebuilder:validation:XValidation:rule="(has(oldSelf.mode) && oldSelf.mode == 'StandBy') ||  ((!has(oldSelf.keyspace) && !has(self.keyspace)) || (has(oldSelf.keyspace) && has(self.keyspace) && oldSelf.keyspace == self.keyspace))",message="keyspace can only be set once when mode is changed from StandBy to Normal"
-	// +kubebuilder:validation:XValidation:rule="!has(self.mode) || self.mode != 'StandBy' || oldSelf.mode == 'StandBy'",message="mode can only be set from StandBy to Normal once"
 	TiDBTemplateSpec `json:",inline"`
 }
 

manifests/crd/core.pingcap.com_tidbs.yaml

@@ -8709,14 +8709,6 @@ spec:
             - message: keyspace cannot be set if mode is StandBy
               rule: '!has(self.mode) || self.mode == ''Normal'' || !has(self.keyspace)
                 || self.keyspace.size() == 0'
-            - message: keyspace can only be set once when mode is changed from StandBy
-                to Normal
-              rule: (has(oldSelf.mode) && oldSelf.mode == 'StandBy') ||  ((!has(oldSelf.keyspace)
-                && !has(self.keyspace)) || (has(oldSelf.keyspace) && has(self.keyspace)
-                && oldSelf.keyspace == self.keyspace))
-            - message: mode can only be set from StandBy to Normal once
-              rule: '!has(self.mode) || self.mode != ''StandBy'' || oldSelf.mode ==
-                ''StandBy'''
             - message: overlay volumeClaims names must exist in volumes
               rule: '!has(self.overlay) || !has(self.overlay.volumeClaims) || (has(self.volumes)
                 && self.overlay.volumeClaims.all(vc, vc.name in self.volumes.map(v,

pkg/configs/tidb/config.go

@@ -83,7 +83,7 @@ type StandBy struct {
 }
 
 func (c *Config) Overlay(cluster *v1alpha1.Cluster, tidb *v1alpha1.TiDB, fg features.Gates) error {
-	if err := c.Validate(coreutil.IsSeparateSlowLogEnabled(tidb)); err != nil {
+	if err := c.Validate(tidb); err != nil {
 		return err
 	}
 
@@ -144,7 +144,8 @@ func (c *Config) Overlay(cluster *v1alpha1.Cluster, tidb *v1alpha1.TiDB, fg feat
 }
 
 //nolint:gocyclo // refactor if possible
-func (c *Config) Validate(separateSlowLog bool) error {
+func (c *Config) Validate(tidb *v1alpha1.TiDB) error {
+	separateSlowLog := coreutil.IsSeparateSlowLogEnabled(tidb)
 	var fields []string
 
 	if c.Store != "" {
@@ -197,8 +198,10 @@ func (c *Config) Validate(separateSlowLog bool) error {
 		fields = append(fields, "labels")
 	}
 
-	if c.KeyspaceName != "" {
-		fields = append(fields, "keyspace")
+	if tidb.Spec.Keyspace != "" {
+		if c.KeyspaceName != "" {
+			fields = append(fields, "keyspace")
+		}
 	}
 
 	if c.StandBy != nil && c.StandBy.StandByMode != nil {

pkg/configs/tidb/config_test.go

@@ -30,7 +30,39 @@ import (
 
 func TestValidate(t *testing.T) {
 	cfgValid := &Config{}
-	err := cfgValid.Validate(true)
+	tidb := &v1alpha1.TiDB{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "ns1",
+			Name:      "basic-0",
+		},
+		Spec: v1alpha1.TiDBSpec{
+			Cluster: v1alpha1.ClusterReference{
+				Name: "cluster-1",
+			},
+			Subdomain: "basic-tidb-peer",
+			TiDBTemplateSpec: v1alpha1.TiDBTemplateSpec{
+				SlowLog: &v1alpha1.TiDBSlowLog{},
+				Security: &v1alpha1.TiDBSecurity{
+					AuthToken: &v1alpha1.TiDBAuthToken{
+						JWKs: corev1.LocalObjectReference{
+							Name: "auth-token-jwks",
+						},
+					},
+					SEM: &v1alpha1.SEM{
+						Config: corev1.LocalObjectReference{
+							Name: "sem-config",
+						},
+					},
+					TLS: &v1alpha1.TiDBTLS{
+						MySQL: &v1alpha1.TLS{
+							Enabled: true,
+						},
+					},
+				},
+			},
+		},
+	}
+	err := cfgValid.Validate(tidb)
 	require.NoError(t, err)
 
 	cfgInvalid := &Config{
@@ -56,7 +88,7 @@ func TestValidate(t *testing.T) {
 		ServerLabels:      map[string]string{"foo": "bar"},
 	}
 
-	err = cfgInvalid.Validate(true)
+	err = cfgInvalid.Validate(tidb)
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "store")
 	assert.Contains(t, err.Error(), "advertise-address")

tests/validation/tidb_test.go

@@ -107,9 +107,7 @@ func mysqlTLS() []Case {
 }
 
 func keyspace() []Case {
-	err0 := `spec: Invalid value: "object": keyspace can only be set once when mode is changed from StandBy to Normal`
 	err1 := `spec: Invalid value: "object": keyspace cannot be set if mode is StandBy`
-	err2 := `spec: Invalid value: "object": mode can only be set from StandBy to Normal once`
 	return []Case{
 		{
 			desc:     "no keyspace and no mode",
@@ -156,24 +154,6 @@ func keyspace() []Case {
 			current: map[string]any{"mode": "Normal", "keyspace": "xxx"},
 			mode:    PatchModeMerge,
 		},
-		{
-			desc:    "update: mode is Normal and try to change keyspace",
-			old:     map[string]any{"mode": "Normal", "keyspace": "xxx"},
-			current: map[string]any{"mode": "Normal", "keyspace": "yyy"},
-			mode:    PatchModeMerge,
-			wantErrs: []string{
-				err0,
-			},
-		},
-		{
-			desc:    "update: mode is Normal and try to unset keyspace",
-			old:     map[string]any{"mode": "Normal", "keyspace": "xxx"},
-			current: map[string]any{"mode": "Normal"},
-			mode:    PatchModeMerge,
-			wantErrs: []string{
-				err0,
-			},
-		},
 		{
 			desc:     "mode is StandBy and no keyspace",
 			isCreate: true,
@@ -228,15 +208,5 @@ func keyspace() []Case {
 			current: map[string]any{"mode": "Normal", "keyspace": "yyy"},
 			mode:    PatchModeMerge,
 		},
-		{
-			desc:    "update: mode is Normal and try to change to StandBy back",
-			old:     map[string]any{"mode": "Normal", "keyspace": "xxx"},
-			current: map[string]any{"mode": "StandBy"},
-			mode:    PatchModeMerge,
-			wantErrs: []string{
-				err0,
-				err2,
-			},
-		},
 	}
 }