fix(group): fix unexpected rolling update when scale out/in (#6053)

Signed-off-by: liubo02 <[email protected]>
pingcap · Jan 26, 2025 · 7d238e5 · 7d238e5
1 parent 2a9af0c
commit 7d238e5
Show file tree

Hide file tree

Showing 35 changed files with 118 additions and 100 deletions.
diff --git a/apis/core/v1alpha1/pd_types.go b/apis/core/v1alpha1/pd_types.go
@@ -111,7 +111,7 @@ func (in *PDGroup) GetDesiredReplicas() int32 {
 }
 
 func (in *PDGroup) GetDesiredVersion() string {
-	return in.Spec.Version
+	return in.Spec.Template.Spec.Version
 }
 
 func (in *PDGroup) GetActualVersion() string {
@@ -276,7 +276,6 @@ func (in *PD) TLSClusterSecretName() string {
 type PDGroupSpec struct {
 	Cluster  ClusterReference `json:"cluster"`
 	Replicas *int32           `json:"replicas"`
-	Version  string           `json:"version"`
 
 	// Bootstrapped means that pd cluster has been bootstrapped,
 	// and there is no need to initialize a new cluster.
@@ -285,6 +284,8 @@ type PDGroupSpec struct {
 	// If it's true, it cannot be set to false for security
 	Bootstrapped bool `json:"bootstrapped,omitempty"`
 
+	// +listType=map
+	// +listMapKey=type
 	SchedulePolicies []SchedulePolicy `json:"schedulePolicies,omitempty"`
 
 	Template PDTemplate `json:"template"`
@@ -298,6 +299,7 @@ type PDTemplate struct {
 // PDTemplateSpec can only be specified in PDGroup
 // TODO: It's name may need to be changed to distinguish from PodTemplateSpec
 type PDTemplateSpec struct {
+	Version string `json:"version"`
 	// Image is pd's image
 	// If tag is omitted, version will be used as the image tag.
 	// Default is pingcap/pd
@@ -344,9 +346,6 @@ type PDSpec struct {
 	// Topology cannot be changed
 	Topology Topology `json:"topology,omitempty"`
 
-	// Version specifies the PD version
-	Version string `json:"version"`
-
 	// Subdomain means the subdomain of the exported pd dns.
 	// A same pd cluster will use a same subdomain
 	Subdomain string `json:"subdomain"`

diff --git a/apis/core/v1alpha1/tidb_types.go b/apis/core/v1alpha1/tidb_types.go
@@ -142,7 +142,7 @@ func (in *TiDBGroup) GetDesiredReplicas() int32 {
 }
 
 func (in *TiDBGroup) GetDesiredVersion() string {
-	return in.Spec.Version
+	return in.Spec.Template.Spec.Version
 }
 
 func (in *TiDBGroup) GetActualVersion() string {
@@ -315,11 +315,12 @@ func (in *TiDB) TLSClusterSecretName() string {
 type TiDBGroupSpec struct {
 	Cluster  ClusterReference `json:"cluster"`
 	Replicas *int32           `json:"replicas"`
-	Version  string           `json:"version"`
 
 	// Service defines some fields used to override the default service.
 	Service *TiDBService `json:"service,omitempty"`
 
+	// +listType=map
+	// +listMapKey=type
 	SchedulePolicies []SchedulePolicy `json:"schedulePolicies,omitempty"`
 
 	Template TiDBTemplate `json:"template"`
@@ -332,6 +333,7 @@ type TiDBTemplate struct {
 
 // TiDBTemplateSpec can only be specified in TiDBGroup.
 type TiDBTemplateSpec struct {
+	Version string `json:"version"`
 	// Image is tidb's image
 	// If tag is omitted, version will be used as the image tag.
 	// Default is pingcap/tidb
@@ -480,9 +482,6 @@ type TiDBSpec struct {
 	// Topology cannot be changed.
 	Topology Topology `json:"topology,omitempty"`
 
-	// Version specifies the TiDB version.
-	Version string `json:"version"`
-
 	// Subdomain means the subdomain of the exported pd dns.
 	// A same pd cluster will use a same subdomain
 	Subdomain string `json:"subdomain"`

diff --git a/apis/core/v1alpha1/tiflash_types.go b/apis/core/v1alpha1/tiflash_types.go
@@ -113,7 +113,7 @@ func (in *TiFlashGroup) GetDesiredReplicas() int32 {
 }
 
 func (in *TiFlashGroup) GetDesiredVersion() string {
-	return in.Spec.Version
+	return in.Spec.Template.Spec.Version
 }
 
 func (in *TiFlashGroup) GetActualVersion() string {
@@ -305,8 +305,9 @@ func (in *TiFlash) TLSClusterSecretName() string {
 type TiFlashGroupSpec struct {
 	Cluster  ClusterReference `json:"cluster"`
 	Replicas *int32           `json:"replicas"`
-	Version  string           `json:"version"`
 
+	// +listType=map
+	// +listMapKey=type
 	SchedulePolicies []SchedulePolicy `json:"schedulePolicies,omitempty"`
 	Template         TiFlashTemplate  `json:"template"`
 }
@@ -317,6 +318,7 @@ type TiFlashTemplate struct {
 }
 
 type TiFlashTemplateSpec struct {
+	Version string `json:"version"`
 	// Image is tiflash's image
 	// If tag is omitted, version will be used as the image tag.
 	// Default is pingcap/tiflash
@@ -382,8 +384,6 @@ type TiFlashSpec struct {
 	// It will be translated into a node affinity config
 	// Topology cannot be changed
 	Topology Topology `json:"topology,omitempty"`
-	// Version specifies the TiFlash version
-	Version string `json:"version"`
 	// Subdomain means the subdomain of the exported TiFlash dns.
 	// A same TiFlash group will use a same subdomain
 	Subdomain string `json:"subdomain"`

diff --git a/apis/core/v1alpha1/tikv_types.go b/apis/core/v1alpha1/tikv_types.go
@@ -148,7 +148,7 @@ func (in *TiKVGroup) GetDesiredReplicas() int32 {
 }
 
 func (in *TiKVGroup) GetDesiredVersion() string {
-	return in.Spec.Version
+	return in.Spec.Template.Spec.Version
 }
 
 func (in *TiKVGroup) GetActualVersion() string {
@@ -281,7 +281,6 @@ func (in *TiKV) TLSClusterSecretName() string {
 type TiKVGroupSpec struct {
 	Cluster  ClusterReference `json:"cluster"`
 	Replicas *int32           `json:"replicas"`
-	Version  string           `json:"version"`
 
 	// +listType=map
 	// +listMapKey=type
@@ -298,6 +297,7 @@ type TiKVTemplate struct {
 // TiKVTemplateSpec can only be specified in TiKVGroup
 // TODO: It's name may need to be changed to distinguish from PodTemplateSpec
 type TiKVTemplateSpec struct {
+	Version string `json:"version"`
 	// Image is tikv's image
 	// If tag is omitted, version will be used as the image tag.
 	// Default is pingcap/tikv
@@ -351,8 +351,6 @@ type TiKVSpec struct {
 	// It will be translated into a node affinity config
 	// Topology cannot be changed
 	Topology Topology `json:"topology,omitempty"`
-	// Version specifies the TiKV version
-	Version string `json:"version"`
 	// Subdomain means the subdomain of the exported tikv dns.
 	// A same tikv group will use a same subdomain
 	Subdomain string `json:"subdomain"`

diff --git a/manifests/crd/core.pingcap.com_pdgroups.yaml b/manifests/crd/core.pingcap.com_pdgroups.yaml
@@ -105,6 +105,9 @@ spec:
                   - type
                   type: object
                 type: array
+                x-kubernetes-list-map-keys:
+                - type
+                x-kubernetes-list-type: map
               template:
                 properties:
                   metadata:
@@ -215,6 +218,8 @@ spec:
                             - HotReload
                             type: string
                         type: object
+                      version:
+                        type: string
                       volumes:
                         description: Volumes defines persistent volumes of PD
                         items:
@@ -276,18 +281,16 @@ spec:
                         type: array
                     required:
                     - config
+                    - version
                     - volumes
                     type: object
                 required:
                 - spec
                 type: object
-              version:
-                type: string
             required:
             - cluster
             - replicas
             - template
-            - version
             type: object
           status:
             properties:

diff --git a/manifests/crd/core.pingcap.com_pds.yaml b/manifests/crd/core.pingcap.com_pds.yaml
@@ -151,7 +151,6 @@ spec:
                     type: string
                 type: object
               version:
-                description: Version specifies the PD version
                 type: string
               volumes:
                 description: Volumes defines persistent volumes of PD

diff --git a/manifests/crd/core.pingcap.com_tidbgroups.yaml b/manifests/crd/core.pingcap.com_tidbgroups.yaml
@@ -97,6 +97,9 @@ spec:
                   - type
                   type: object
                 type: array
+                x-kubernetes-list-map-keys:
+                - type
+                x-kubernetes-list-type: map
               service:
                 description: Service defines some fields used to override the default
                   service.
@@ -330,6 +333,8 @@ spec:
                             - HotReload
                             type: string
                         type: object
+                      version:
+                        type: string
                       volumes:
                         description: Volumes defines data volume of TiDB, it is optional.
                         items:
@@ -391,17 +396,15 @@ spec:
                         type: array
                     required:
                     - config
+                    - version
                     type: object
                 required:
                 - spec
                 type: object
-              version:
-                type: string
             required:
             - cluster
             - replicas
             - template
-            - version
             type: object
           status:
             properties:

diff --git a/manifests/crd/core.pingcap.com_tidbs.yaml b/manifests/crd/core.pingcap.com_tidbs.yaml
@@ -242,7 +242,6 @@ spec:
                     type: string
                 type: object
               version:
-                description: Version specifies the TiDB version.
                 type: string
               volumes:
                 description: Volumes defines data volume of TiDB, it is optional.

diff --git a/manifests/crd/core.pingcap.com_tiflashes.yaml b/manifests/crd/core.pingcap.com_tiflashes.yaml
@@ -195,7 +195,6 @@ spec:
                     type: string
                 type: object
               version:
-                description: Version specifies the TiFlash version
                 type: string
               volumes:
                 description: Volumes defines data volume of TiFlash

diff --git a/manifests/crd/core.pingcap.com_tiflashgroups.yaml b/manifests/crd/core.pingcap.com_tiflashgroups.yaml
@@ -96,6 +96,9 @@ spec:
                   - type
                   type: object
                 type: array
+                x-kubernetes-list-map-keys:
+                - type
+                x-kubernetes-list-type: map
               template:
                 properties:
                   metadata:
@@ -250,6 +253,8 @@ spec:
                             - HotReload
                             type: string
                         type: object
+                      version:
+                        type: string
                       volumes:
                         description: Volumes defines data volume of TiFlash
                         items:
@@ -311,18 +316,16 @@ spec:
                         type: array
                     required:
                     - config
+                    - version
                     - volumes
                     type: object
                 required:
                 - spec
                 type: object
-              version:
-                type: string
             required:
             - cluster
             - replicas
             - template
-            - version
             type: object
           status:
             properties:

diff --git a/manifests/crd/core.pingcap.com_tikvgroups.yaml b/manifests/crd/core.pingcap.com_tikvgroups.yaml
@@ -215,6 +215,8 @@ spec:
                             - HotReload
                             type: string
                         type: object
+                      version:
+                        type: string
                       volumes:
                         description: Volumes defines data volume of TiKV
                         items:
@@ -276,18 +278,16 @@ spec:
                         type: array
                     required:
                     - config
+                    - version
                     - volumes
                     type: object
                 required:
                 - spec
                 type: object
-              version:
-                type: string
             required:
             - cluster
             - replicas
             - template
-            - version
             type: object
           status:
             properties:

diff --git a/manifests/crd/core.pingcap.com_tikvs.yaml b/manifests/crd/core.pingcap.com_tikvs.yaml
@@ -155,7 +155,6 @@ spec:
                     type: string
                 type: object
               version:
-                description: Version specifies the TiKV version
                 type: string
               volumes:
                 description: Volumes defines data volume of TiKV

diff --git a/pkg/action/upgrader_test.go b/pkg/action/upgrader_test.go
@@ -201,7 +201,11 @@ func TestUpgradePolicy(t *testing.T) {
 			Cluster: v1alpha1.ClusterReference{
 				Name: "tc",
 			},
-			Version: "v8.5.0",
+			Template: v1alpha1.PDTemplate{
+				Spec: v1alpha1.PDTemplateSpec{
+					Version: "v8.5.0",
+				},
+			},
 		},
 		Status: v1alpha1.PDGroupStatus{
 			GroupStatus: v1alpha1.GroupStatus{
@@ -222,7 +226,11 @@ func TestUpgradePolicy(t *testing.T) {
 			Cluster: v1alpha1.ClusterReference{
 				Name: "tc",
 			},
-			Version: "v8.5.0",
+			Template: v1alpha1.TiKVTemplate{
+				Spec: v1alpha1.TiKVTemplateSpec{
+					Version: "v8.5.0",
+				},
+			},
 		},
 		Status: v1alpha1.TiKVGroupStatus{
 			GroupStatus: v1alpha1.GroupStatus{

diff --git a/pkg/controllers/common/revision_test.go b/pkg/controllers/common/revision_test.go
@@ -46,8 +46,8 @@ func (f *fakeRevisionStateInitializer[G]) RevisionInitializer() RevisionInitiali
 }
 
 const (
-	fakeOldRevision = "aaa-c9f48df69"
-	fakeNewRevision = "aaa-c9f48df68"
+	fakeOldRevision = "aaa-77554fbd78"
+	fakeNewRevision = "aaa-77554fbd79"
 )
 
 func TestTaskRevision(t *testing.T) {
@@ -169,7 +169,7 @@ func TestTaskRevision(t *testing.T) {
 				fake.FakeObj(fakeOldRevision, fake.Label[appsv1.ControllerRevision]("ccc", "ddd")),
 				fake.FakeObj(fakeNewRevision, fake.Label[appsv1.ControllerRevision]("ccc", "ddd")),
 			},
-			expectedUpdateRevision:  "aaa-c9f48df6c",
+			expectedUpdateRevision:  "aaa-77554fbd76",
 			expectedCurrentRevision: fakeOldRevision,
 			expectedCollisionCount:  2,
 		},

diff --git a/pkg/controllers/pdgroup/tasks/updater.go b/pkg/controllers/pdgroup/tasks/updater.go
@@ -99,7 +99,7 @@ func TaskUpdater(state *ReconcileContext, c client.Client) task.Task {
 }
 
 func needVersionUpgrade(pdg *v1alpha1.PDGroup) bool {
-	return pdg.Spec.Version != pdg.Status.Version && pdg.Status.Version != ""
+	return pdg.Spec.Template.Spec.Version != pdg.Status.Version && pdg.Status.Version != ""
 }
 
 const (
@@ -141,7 +141,6 @@ func PDNewer(pdg *v1alpha1.PDGroup, rev string) updater.NewFactory[*runtime.PD]
 			},
 			Spec: v1alpha1.PDSpec{
 				Cluster:        pdg.Spec.Cluster,
-				Version:        pdg.Spec.Version,
 				Subdomain:      HeadlessServiceName(pdg.Name),
 				PDTemplateSpec: *spec,
 			},