From 7d238e5e63baf86ab9ecddabaf0701e6b267f67d Mon Sep 17 00:00:00 2001 From: Bo Liu Date: Sun, 26 Jan 2025 16:33:10 +0800 Subject: [PATCH] fix(group): fix unexpected rolling update when scale out/in (#6053) Signed-off-by: liubo02 --- apis/core/v1alpha1/pd_types.go | 9 ++--- apis/core/v1alpha1/tidb_types.go | 9 ++--- apis/core/v1alpha1/tiflash_types.go | 8 ++-- apis/core/v1alpha1/tikv_types.go | 6 +-- manifests/crd/core.pingcap.com_pdgroups.yaml | 9 +++-- manifests/crd/core.pingcap.com_pds.yaml | 1 - .../crd/core.pingcap.com_tidbgroups.yaml | 9 +++-- manifests/crd/core.pingcap.com_tidbs.yaml | 1 - manifests/crd/core.pingcap.com_tiflashes.yaml | 1 - .../crd/core.pingcap.com_tiflashgroups.yaml | 9 +++-- .../crd/core.pingcap.com_tikvgroups.yaml | 6 +-- manifests/crd/core.pingcap.com_tikvs.yaml | 1 - pkg/action/upgrader_test.go | 12 +++++- pkg/controllers/common/revision_test.go | 6 +-- pkg/controllers/pdgroup/tasks/updater.go | 3 +- pkg/controllers/pdgroup/tasks/updater_test.go | 2 +- pkg/controllers/tidbgroup/tasks/state_test.go | 4 +- pkg/controllers/tidbgroup/tasks/updater.go | 3 +- .../tidbgroup/tasks/updater_test.go | 2 +- .../tiflashgroup/tasks/state_test.go | 4 +- pkg/controllers/tiflashgroup/tasks/updater.go | 5 +-- .../tiflashgroup/tasks/updater_test.go | 2 +- pkg/controllers/tikvgroup/tasks/updater.go | 3 +- .../tikvgroup/tasks/updater_test.go | 2 +- pkg/runtime/pd.go | 2 +- pkg/runtime/tidb.go | 2 +- pkg/runtime/tiflash.go | 2 +- pkg/runtime/tikv.go | 2 +- pkg/utils/k8s/revision/controller_revision.go | 11 ++++-- .../k8s/revision/controller_revision_test.go | 38 +++++++++++-------- tests/e2e/cluster/cluster.go | 16 ++++---- tests/e2e/data/pd.go | 4 +- tests/e2e/data/tidb.go | 4 +- tests/e2e/data/tikv.go | 4 +- tests/e2e/utils/data/cluster.go | 16 ++++---- 35 files changed, 118 insertions(+), 100 deletions(-) diff --git a/apis/core/v1alpha1/pd_types.go b/apis/core/v1alpha1/pd_types.go index d0f7b4d378..6dde33c71b 100644 --- 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 index 58de0d0b06..2e0fb43bc7 100644 --- 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 index bcdeeed8ac..0780b6fc80 100644 --- 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 index a1a20c620e..13bdd76438 100644 --- 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 index 50548a4abe..7471a10d5b 100644 --- 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 index 9b97297e6d..cd801f5769 100644 --- 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 index 9b406929ce..94eb1ed73c 100644 --- 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 index db7a1b1237..ad6d93d660 100644 --- 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 index 143188fcfb..e4ba20aa46 100644 --- 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 index 609ed40703..2c133435c6 100644 --- 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 index e41751bff1..6e24f7529d 100644 --- 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 index ef6d77206d..9aa4cd6139 100644 --- 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 index fd7c1fe219..56c056bdf5 100644 --- 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 index 02b0d2b7fb..ebe064652b 100644 --- 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 index fe260f1ebc..d6c623dfb6 100644 --- 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, }, diff --git a/pkg/controllers/pdgroup/tasks/updater_test.go b/pkg/controllers/pdgroup/tasks/updater_test.go index c962f72024..7ec9c59bbf 100644 --- a/pkg/controllers/pdgroup/tasks/updater_test.go +++ b/pkg/controllers/pdgroup/tasks/updater_test.go @@ -65,7 +65,7 @@ func TestTaskUpdater(t *testing.T) { pdg: fake.FakeObj("aaa", func(obj *v1alpha1.PDGroup) *v1alpha1.PDGroup { // use an wrong version to trigger version check // TODO(liubo02): it's not happened actually. Maybe remove whole checking - obj.Spec.Version = "xxx" + obj.Spec.Template.Spec.Version = "xxx" obj.Status.Version = "yyy" return obj }), diff --git a/pkg/controllers/tidbgroup/tasks/state_test.go b/pkg/controllers/tidbgroup/tasks/state_test.go index f8da6f9b23..314b7419e0 100644 --- a/pkg/controllers/tidbgroup/tasks/state_test.go +++ b/pkg/controllers/tidbgroup/tasks/state_test.go @@ -79,8 +79,8 @@ func TestState(t *testing.T) { return obj }), }, - updateRevision: "aaa-659f78b499", - currentRevision: "aaa-659f78b499", + updateRevision: "aaa-dfb684447", + currentRevision: "aaa-dfb684447", }, }, } diff --git a/pkg/controllers/tidbgroup/tasks/updater.go b/pkg/controllers/tidbgroup/tasks/updater.go index e630390681..349fb9d179 100644 --- a/pkg/controllers/tidbgroup/tasks/updater.go +++ b/pkg/controllers/tidbgroup/tasks/updater.go @@ -94,7 +94,7 @@ func TaskUpdater(state *ReconcileContext, c client.Client) task.Task { } func needVersionUpgrade(dbg *v1alpha1.TiDBGroup) bool { - return dbg.Spec.Version != dbg.Status.Version && dbg.Status.Version != "" + return dbg.Spec.Template.Spec.Version != dbg.Status.Version && dbg.Status.Version != "" } const ( @@ -124,7 +124,6 @@ func TiDBNewer(dbg *v1alpha1.TiDBGroup, rev string) updater.NewFactory[*runtime. }, Spec: v1alpha1.TiDBSpec{ Cluster: dbg.Spec.Cluster, - Version: dbg.Spec.Version, Subdomain: HeadlessServiceName(dbg.Name), // same as headless service TiDBTemplateSpec: *spec, }, diff --git a/pkg/controllers/tidbgroup/tasks/updater_test.go b/pkg/controllers/tidbgroup/tasks/updater_test.go index 2afb0f626e..256e0ac85a 100644 --- a/pkg/controllers/tidbgroup/tasks/updater_test.go +++ b/pkg/controllers/tidbgroup/tasks/updater_test.go @@ -65,7 +65,7 @@ func TestTaskUpdater(t *testing.T) { dbg: fake.FakeObj("aaa", func(obj *v1alpha1.TiDBGroup) *v1alpha1.TiDBGroup { // use an wrong version to trigger version check // TODO(liubo02): it's not happened actually. Maybe remove whole checking - obj.Spec.Version = "xxx" + obj.Spec.Template.Spec.Version = "xxx" obj.Status.Version = "yyy" return obj }), diff --git a/pkg/controllers/tiflashgroup/tasks/state_test.go b/pkg/controllers/tiflashgroup/tasks/state_test.go index 4e4393543c..15715ea40b 100644 --- a/pkg/controllers/tiflashgroup/tasks/state_test.go +++ b/pkg/controllers/tiflashgroup/tasks/state_test.go @@ -79,8 +79,8 @@ func TestState(t *testing.T) { return obj }), }, - updateRevision: "aaa-566bb4bcfd", - currentRevision: "aaa-566bb4bcfd", + updateRevision: "aaa-77554fbd78", + currentRevision: "aaa-77554fbd78", }, }, } diff --git a/pkg/controllers/tiflashgroup/tasks/updater.go b/pkg/controllers/tiflashgroup/tasks/updater.go index cedd08f42f..5ebfdd1be3 100644 --- a/pkg/controllers/tiflashgroup/tasks/updater.go +++ b/pkg/controllers/tiflashgroup/tasks/updater.go @@ -93,8 +93,8 @@ func TaskUpdater(state *ReconcileContext, c client.Client) task.Task { }) } -func needVersionUpgrade(flashg *v1alpha1.TiFlashGroup) bool { - return flashg.Spec.Version != flashg.Status.Version && flashg.Status.Version != "" +func needVersionUpgrade(fg *v1alpha1.TiFlashGroup) bool { + return fg.Spec.Template.Spec.Version != fg.Status.Version && fg.Status.Version != "" } const ( @@ -124,7 +124,6 @@ func TiFlashNewer(fg *v1alpha1.TiFlashGroup, rev string) updater.NewFactory[*run }, Spec: v1alpha1.TiFlashSpec{ Cluster: fg.Spec.Cluster, - Version: fg.Spec.Version, Subdomain: HeadlessServiceName(fg.Name), TiFlashTemplateSpec: *spec, }, diff --git a/pkg/controllers/tiflashgroup/tasks/updater_test.go b/pkg/controllers/tiflashgroup/tasks/updater_test.go index 3293e462ba..4b913d1adc 100644 --- a/pkg/controllers/tiflashgroup/tasks/updater_test.go +++ b/pkg/controllers/tiflashgroup/tasks/updater_test.go @@ -65,7 +65,7 @@ func TestTaskUpdater(t *testing.T) { fg: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlashGroup) *v1alpha1.TiFlashGroup { // use an wrong version to trigger version check // TODO(liubo02): it's not happened actually. Maybe remove whole checking - obj.Spec.Version = "xxx" + obj.Spec.Template.Spec.Version = "xxx" obj.Status.Version = "yyy" return obj }), diff --git a/pkg/controllers/tikvgroup/tasks/updater.go b/pkg/controllers/tikvgroup/tasks/updater.go index 0d465670ad..588f84f69c 100644 --- a/pkg/controllers/tikvgroup/tasks/updater.go +++ b/pkg/controllers/tikvgroup/tasks/updater.go @@ -95,7 +95,7 @@ func TaskUpdater(state *ReconcileContext, c client.Client) task.Task { } func needVersionUpgrade(kvg *v1alpha1.TiKVGroup) bool { - return kvg.Spec.Version != kvg.Status.Version && kvg.Status.Version != "" + return kvg.Spec.Template.Spec.Version != kvg.Status.Version && kvg.Status.Version != "" } const ( @@ -126,7 +126,6 @@ func TiKVNewer(kvg *v1alpha1.TiKVGroup, rev string) updater.NewFactory[*runtime. }, Spec: v1alpha1.TiKVSpec{ Cluster: kvg.Spec.Cluster, - Version: kvg.Spec.Version, Subdomain: HeadlessServiceName(kvg.Name), TiKVTemplateSpec: *spec, }, diff --git a/pkg/controllers/tikvgroup/tasks/updater_test.go b/pkg/controllers/tikvgroup/tasks/updater_test.go index 3367ed1fbc..1ede0c541c 100644 --- a/pkg/controllers/tikvgroup/tasks/updater_test.go +++ b/pkg/controllers/tikvgroup/tasks/updater_test.go @@ -65,7 +65,7 @@ func TestTaskUpdater(t *testing.T) { kvg: fake.FakeObj("aaa", func(obj *v1alpha1.TiKVGroup) *v1alpha1.TiKVGroup { // use an wrong version to trigger version check // TODO(liubo02): it's not happened actually. Maybe remove whole checking - obj.Spec.Version = "xxx" + obj.Spec.Template.Spec.Version = "xxx" obj.Status.Version = "yyy" return obj }), diff --git a/pkg/runtime/pd.go b/pkg/runtime/pd.go index a008eb21ff..7d53984543 100644 --- a/pkg/runtime/pd.go +++ b/pkg/runtime/pd.go @@ -188,7 +188,7 @@ func (pdg *PDGroup) Replicas() int32 { } func (pdg *PDGroup) Version() string { - return pdg.Spec.Version + return pdg.Spec.Template.Spec.Version } func (pdg *PDGroup) Cluster() string { diff --git a/pkg/runtime/tidb.go b/pkg/runtime/tidb.go index 4d5853656d..24773e5f2f 100644 --- a/pkg/runtime/tidb.go +++ b/pkg/runtime/tidb.go @@ -188,7 +188,7 @@ func (dbg *TiDBGroup) Replicas() int32 { } func (dbg *TiDBGroup) Version() string { - return dbg.Spec.Version + return dbg.Spec.Template.Spec.Version } func (dbg *TiDBGroup) Cluster() string { diff --git a/pkg/runtime/tiflash.go b/pkg/runtime/tiflash.go index 7ba0968d5b..18c90406ea 100644 --- a/pkg/runtime/tiflash.go +++ b/pkg/runtime/tiflash.go @@ -188,7 +188,7 @@ func (fg *TiFlashGroup) Replicas() int32 { } func (fg *TiFlashGroup) Version() string { - return fg.Spec.Version + return fg.Spec.Template.Spec.Version } func (fg *TiFlashGroup) Cluster() string { diff --git a/pkg/runtime/tikv.go b/pkg/runtime/tikv.go index 71b98a7f8b..b4bae93954 100644 --- a/pkg/runtime/tikv.go +++ b/pkg/runtime/tikv.go @@ -188,7 +188,7 @@ func (kvg *TiKVGroup) Replicas() int32 { } func (kvg *TiKVGroup) Version() string { - return kvg.Spec.Version + return kvg.Spec.Template.Spec.Version } func (kvg *TiKVGroup) Cluster() string { diff --git a/pkg/utils/k8s/revision/controller_revision.go b/pkg/utils/k8s/revision/controller_revision.go index d46420d86e..9730539db7 100644 --- a/pkg/utils/k8s/revision/controller_revision.go +++ b/pkg/utils/k8s/revision/controller_revision.go @@ -163,9 +163,14 @@ func getPatch(obj client.Object, gvk schema.GroupVersionKind) ([]byte, error) { } objCopy := make(map[string]any) - objCopy["$patch"] = "replace" - objCopy["spec"] = raw["spec"].(map[string]any) - return json.Marshal(objCopy) + specCopy := make(map[string]any) + spec := raw["spec"].(map[string]any) + template := spec["template"].(map[string]any) + specCopy["template"] = template + template["$patch"] = "replace" + objCopy["spec"] = specCopy + patch, err := json.Marshal(objCopy) + return patch, err } // TruncateHistory truncates any non-live ControllerRevisions in revisions from group's history. diff --git a/pkg/utils/k8s/revision/controller_revision_test.go b/pkg/utils/k8s/revision/controller_revision_test.go index 4c6a67d03e..8f3beb19e8 100644 --- a/pkg/utils/k8s/revision/controller_revision_test.go +++ b/pkg/utils/k8s/revision/controller_revision_test.go @@ -207,14 +207,22 @@ func TestGetCurrentAndUpdate(t *testing.T) { Replicas: ptr.To[int32](1), }, } - rev1, err := newRevision(pdg, "pd", nil, pdg.GVK(), 1, ptr.To[int32](0)) + rev1, err := newRevision(pdg, "pd", nil, pdg.GVK(), 0, ptr.To[int32](0)) require.NoError(t, err) pdg2 := pdg.DeepCopy() - pdg2.Spec.Replicas = ptr.To[int32](2) - rev2, err := newRevision(pdg2, "pd", nil, pdg2.GVK(), 2, ptr.To[int32](0)) + pdg2.Spec.Replicas = ptr.To[int32](3) + rev2, err := newRevision(pdg2, "pd", nil, pdg2.GVK(), 1, ptr.To[int32](0)) require.NoError(t, err) + pdg3 := pdg.DeepCopy() + pdg3.Spec.Template.Spec.Version = "xxx" + rev3, err := newRevision(pdg3, "pd", nil, pdg3.GVK(), 2, ptr.To[int32](0)) + require.NoError(t, err) + + require.Equal(t, rev1.Name, rev2.Name) + require.NotEqual(t, rev1.Name, rev3.Name) + tests := []struct { name string group client.Object @@ -229,26 +237,26 @@ func TestGetCurrentAndUpdate(t *testing.T) { group: pdg, revisions: []*appsv1.ControllerRevision{}, accessor: pdg, - expectedCurRev: "basic-pd-687bcf9d45", - expectedUpdRev: "basic-pd-687bcf9d45", + expectedCurRev: rev1.Name, + expectedUpdRev: rev1.Name, expectedErr: false, }, { - name: "match the prior revision", + name: "change replicas, no revision change", group: pdg2, - revisions: []*appsv1.ControllerRevision{rev1, rev2}, + revisions: []*appsv1.ControllerRevision{rev1}, accessor: pdg2, - expectedCurRev: "basic-pd-5f5f578c9d", - expectedUpdRev: "basic-pd-5f5f578c9d", + expectedCurRev: rev1.Name, + expectedUpdRev: rev1.Name, expectedErr: false, }, { - name: "match an earlier revision", - group: pdg, - revisions: []*appsv1.ControllerRevision{rev1, rev2}, - accessor: pdg, - expectedCurRev: "basic-pd-687bcf9d45", - expectedUpdRev: "basic-pd-687bcf9d45", + name: "change template", + group: pdg3, + revisions: []*appsv1.ControllerRevision{rev1}, + accessor: pdg3, + expectedCurRev: rev3.Name, + expectedUpdRev: rev3.Name, expectedErr: false, }, } diff --git a/tests/e2e/cluster/cluster.go b/tests/e2e/cluster/cluster.go index 0289d2da3b..a35741fcbd 100644 --- a/tests/e2e/cluster/cluster.go +++ b/tests/e2e/cluster/cluster.go @@ -1158,7 +1158,7 @@ var _ = Describe("TiDB Cluster", func() { By("Checking the version of tikv group") var kvgGet v1alpha1.TiKVGroup Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: tc.Namespace, Name: kvg.Name}, &kvgGet)).To(Succeed()) - oldVersion := kvgGet.Spec.Version + oldVersion := kvgGet.Spec.Template.Spec.Version Expect(oldVersion).NotTo(BeEmpty()) Expect(kvgGet.Status.Version).To(Equal(oldVersion)) v, err := semver.NewVersion(oldVersion) @@ -1166,7 +1166,7 @@ var _ = Describe("TiDB Cluster", func() { newVersion := "v" + v.IncMinor().String() By(fmt.Sprintf("Updating the version of the tikv group from %s to %s", oldVersion, newVersion)) - kvgGet.Spec.Version = newVersion + kvgGet.Spec.Template.Spec.Version = newVersion Expect(k8sClient.Update(ctx, &kvgGet)).To(Succeed()) Consistently(func(g Gomega) { @@ -1177,7 +1177,7 @@ var _ = Describe("TiDB Cluster", func() { // tikv should not be upgraded g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: tc.Namespace, Name: kvg.Name}, &kvgGet)).To(Succeed()) - g.Expect(kvgGet.Spec.Version).To(Equal(newVersion)) + g.Expect(kvgGet.Spec.Template.Spec.Version).To(Equal(newVersion)) g.Expect(kvgGet.Status.Version).To(Equal(oldVersion)) }).WithTimeout(1 * time.Minute).WithPolling(createClusterPolling).Should(Succeed()) @@ -1190,15 +1190,15 @@ var _ = Describe("TiDB Cluster", func() { By("Upgrading the pd group") var pdgGet v1alpha1.PDGroup Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: tc.Namespace, Name: pdg.Name}, &pdgGet)).To(Succeed()) - Expect(pdgGet.Spec.Version).To(Equal(oldVersion)) + Expect(pdgGet.Spec.Template.Spec.Version).To(Equal(oldVersion)) Expect(pdgGet.Status.Version).To(Equal(oldVersion)) - pdgGet.Spec.Version = newVersion + pdgGet.Spec.Template.Spec.Version = newVersion Expect(k8sClient.Update(ctx, &pdgGet)).To(Succeed()) By("Checking the version of pd group when paused") Consistently(func(g Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: tc.Namespace, Name: pdg.Name}, &pdgGet)).To(Succeed()) - g.Expect(pdgGet.Spec.Version).To(Equal(newVersion)) + g.Expect(pdgGet.Spec.Template.Spec.Version).To(Equal(newVersion)) g.Expect(pdgGet.Status.Version).To(Equal(oldVersion)) }).WithTimeout(1 * time.Minute).WithPolling(createClusterPolling).Should(Succeed()) @@ -1215,7 +1215,7 @@ var _ = Describe("TiDB Cluster", func() { // pd and tikv should be upgraded g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: tc.Namespace, Name: pdg.Name}, &pdgGet)).To(Succeed()) - g.Expect(pdgGet.Spec.Version).To(Equal(newVersion)) + g.Expect(pdgGet.Spec.Template.Spec.Version).To(Equal(newVersion)) g.Expect(pdgGet.Status.Version).To(Equal(newVersion)) var pdPods corev1.PodList g.Expect(k8sClient.List(ctx, &pdPods, client.InNamespace(tc.Namespace), @@ -1229,7 +1229,7 @@ var _ = Describe("TiDB Cluster", func() { } g.Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: tc.Namespace, Name: kvg.Name}, &kvgGet)).To(Succeed()) - g.Expect(kvgGet.Spec.Version).To(Equal(newVersion)) + g.Expect(kvgGet.Spec.Template.Spec.Version).To(Equal(newVersion)) g.Expect(kvgGet.Status.Version).To(Equal(newVersion)) g.Expect(k8sClient.List(ctx, &pdPods, client.InNamespace(tc.Namespace), client.MatchingLabels{v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiKV})) diff --git a/tests/e2e/data/pd.go b/tests/e2e/data/pd.go index 49890b038d..52fe8097fc 100644 --- a/tests/e2e/data/pd.go +++ b/tests/e2e/data/pd.go @@ -31,11 +31,11 @@ func NewPDGroup(ns string, patches ...GroupPatch[*runtime.PDGroup]) *v1alpha1.PD }, Spec: v1alpha1.PDGroupSpec{ Cluster: v1alpha1.ClusterReference{Name: defaultClusterName}, - Version: defaultVersion, Replicas: ptr.To[int32](1), Template: v1alpha1.PDTemplate{ Spec: v1alpha1.PDTemplateSpec{ - Image: ptr.To(defaultImageRegistry + "pd"), + Version: defaultVersion, + Image: ptr.To(defaultImageRegistry + "pd"), Volumes: []v1alpha1.Volume{ { Name: "data", diff --git a/tests/e2e/data/tidb.go b/tests/e2e/data/tidb.go index fe68b3edb6..e1e742aa0e 100644 --- a/tests/e2e/data/tidb.go +++ b/tests/e2e/data/tidb.go @@ -35,11 +35,11 @@ func NewTiDBGroup(ns string, patches ...GroupPatch[*runtime.TiDBGroup]) *v1alpha }, Spec: v1alpha1.TiDBGroupSpec{ Cluster: v1alpha1.ClusterReference{Name: defaultClusterName}, - Version: defaultVersion, Replicas: ptr.To[int32](1), Template: v1alpha1.TiDBTemplate{ Spec: v1alpha1.TiDBTemplateSpec{ - Image: ptr.To(defaultImageRegistry + "tidb"), + Version: defaultVersion, + Image: ptr.To(defaultImageRegistry + "tidb"), SlowLog: &v1alpha1.TiDBSlowLog{ Image: ptr.To(defaultHelperImage), }, diff --git a/tests/e2e/data/tikv.go b/tests/e2e/data/tikv.go index 5b7b8838ca..166a871e19 100644 --- a/tests/e2e/data/tikv.go +++ b/tests/e2e/data/tikv.go @@ -32,10 +32,10 @@ func NewTiKVGroup(ns string, patches ...GroupPatch[*runtime.TiKVGroup]) *v1alpha Spec: v1alpha1.TiKVGroupSpec{ Cluster: v1alpha1.ClusterReference{Name: defaultClusterName}, Replicas: ptr.To[int32](1), - Version: defaultVersion, Template: v1alpha1.TiKVTemplate{ Spec: v1alpha1.TiKVTemplateSpec{ - Image: ptr.To(defaultImageRegistry + "tikv"), + Version: defaultVersion, + Image: ptr.To(defaultImageRegistry + "tikv"), Volumes: []v1alpha1.Volume{ { Name: "data", diff --git a/tests/e2e/utils/data/cluster.go b/tests/e2e/utils/data/cluster.go index 7d1a4287f4..d4f69b74c3 100644 --- a/tests/e2e/utils/data/cluster.go +++ b/tests/e2e/utils/data/cluster.go @@ -61,10 +61,10 @@ func NewPDGroup(namespace, name, clusterName string, replicas *int32, apply func Spec: v1alpha1.PDGroupSpec{ Cluster: v1alpha1.ClusterReference{Name: clusterName}, Replicas: replicas, - Version: version, Template: v1alpha1.PDTemplate{ Spec: v1alpha1.PDTemplateSpec{ - Image: ptr.To(imageRegistry + "pd"), + Version: version, + Image: ptr.To(imageRegistry + "pd"), Volumes: []v1alpha1.Volume{ { Name: "data", @@ -95,11 +95,11 @@ func NewTiKVGroup(namespace, name, clusterName string, replicas *int32, apply fu }, Spec: v1alpha1.TiKVGroupSpec{ Cluster: v1alpha1.ClusterReference{Name: clusterName}, - Version: version, Replicas: replicas, Template: v1alpha1.TiKVTemplate{ Spec: v1alpha1.TiKVTemplateSpec{ - Image: ptr.To(imageRegistry + "tikv"), + Version: version, + Image: ptr.To(imageRegistry + "tikv"), Volumes: []v1alpha1.Volume{ { Name: "data", @@ -130,11 +130,11 @@ func NewTiDBGroup(namespace, name, clusterName string, replicas *int32, apply fu }, Spec: v1alpha1.TiDBGroupSpec{ Cluster: v1alpha1.ClusterReference{Name: clusterName}, - Version: version, Replicas: replicas, Template: v1alpha1.TiDBTemplate{ Spec: v1alpha1.TiDBTemplateSpec{ - Image: ptr.To(imageRegistry + "tidb"), + Version: version, + Image: ptr.To(imageRegistry + "tidb"), SlowLog: &v1alpha1.TiDBSlowLog{ Image: ptr.To(helperImage), }, @@ -163,11 +163,11 @@ func NewTiFlashGroup(namespace, name, clusterName string, replicas *int32, }, Spec: v1alpha1.TiFlashGroupSpec{ Cluster: v1alpha1.ClusterReference{Name: clusterName}, - Version: version, Replicas: replicas, Template: v1alpha1.TiFlashTemplate{ Spec: v1alpha1.TiFlashTemplateSpec{ - Image: ptr.To(imageRegistry + "tiflash"), + Version: version, + Image: ptr.To(imageRegistry + "tiflash"), Volumes: []v1alpha1.Volume{ { Name: "data",