Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions apis/core/v1alpha1/pd_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -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"`
Expand All @@ -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
Expand Down Expand Up @@ -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"`
Expand Down
9 changes: 4 additions & 5 deletions apis/core/v1alpha1/tidb_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"`
Expand All @@ -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
Expand Down Expand Up @@ -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"`
Expand Down
8 changes: 4 additions & 4 deletions apis/core/v1alpha1/tiflash_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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"`
}
Expand All @@ -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
Expand Down Expand Up @@ -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"`
Expand Down
6 changes: 2 additions & 4 deletions apis/core/v1alpha1/tikv_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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"`
Expand Down
9 changes: 6 additions & 3 deletions manifests/crd/core.pingcap.com_pdgroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ spec:
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
template:
properties:
metadata:
Expand Down Expand Up @@ -215,6 +218,8 @@ spec:
- HotReload
type: string
type: object
version:
type: string
volumes:
description: Volumes defines persistent volumes of PD
items:
Expand Down Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion manifests/crd/core.pingcap.com_pds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions manifests/crd/core.pingcap.com_tidbgroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion manifests/crd/core.pingcap.com_tidbs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion manifests/crd/core.pingcap.com_tiflashes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions manifests/crd/core.pingcap.com_tiflashgroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ spec:
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
template:
properties:
metadata:
Expand Down Expand Up @@ -250,6 +253,8 @@ spec:
- HotReload
type: string
type: object
version:
type: string
volumes:
description: Volumes defines data volume of TiFlash
items:
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions manifests/crd/core.pingcap.com_tikvgroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ spec:
- HotReload
type: string
type: object
version:
type: string
volumes:
description: Volumes defines data volume of TiKV
items:
Expand Down Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion manifests/crd/core.pingcap.com_tikvs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions pkg/action/upgrader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/common/revision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
},
Expand Down
3 changes: 1 addition & 2 deletions pkg/controllers/pdgroup/tasks/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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,
},
Expand Down
Loading
Loading