Skip to content
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
3 changes: 1 addition & 2 deletions pkg/controller/instanceset2/instance_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ func parseParentNameAndOrdinal(s string) (string, int) {
// reverse it if reverse==true
func sortObjects[T client.Object](objects []T, rolePriorityMap map[string]int, reverse bool) {
getRolePriorityFunc := func(i int) int {
role := strings.ToLower(objects[i].GetLabels()[constant.RoleLabelKey])
return rolePriorityMap[role]
return getRolePriority(rolePriorityMap, objects[i].GetLabels()[constant.RoleLabelKey])
}

// cache the parent names and ordinals to accelerate the parsing process when there is a massive number of Pods.
Expand Down
17 changes: 17 additions & 0 deletions pkg/controller/instanceset2/instance_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,23 @@ func TestSortObjects(t *testing.T) {
}
}

func TestSortInstancesNormalizesObservedRoleName(t *testing.T) {
roles := []workloads.ReplicaRole{
{Name: "Follower", UpdatePriority: 1},
{Name: "Leader", UpdatePriority: 2},
}
priorityMap := composeRolePriorityMap(roles)
instances := []workloads.Instance{
{ObjectMeta: metav1.ObjectMeta{Name: "mysql-0"}, Status: workloads.InstanceStatus2{Role: "Follower"}},
{ObjectMeta: metav1.ObjectMeta{Name: "mysql-1"}, Status: workloads.InstanceStatus2{Role: "Leader"}},
}

sortInstances(instances, priorityMap, false)
Comment thread
leon-ape marked this conversation as resolved.
if got := instances[0].Name; got != "mysql-0" {
t.Fatalf("sorted instances = %s, want mysql-0", got)
}
}

func TestCopyAndMergeService(t *testing.T) {
oldSvc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Expand Down
13 changes: 7 additions & 6 deletions pkg/controller/instanceset2/update_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,12 @@ func (p *realUpdatePlan) buildBestEffortParallelUpdatePlan(rolePriorityMap map[s
quorumPriority := math.MaxInt32
leaderPriority := 0
for _, role := range p.its.Spec.Roles {
if rolePriorityMap[role.Name] > leaderPriority {
leaderPriority = rolePriorityMap[role.Name]
rolePriority := getRolePriority(rolePriorityMap, role.Name)
if rolePriority > leaderPriority {
leaderPriority = rolePriority
}
if role.ParticipatesInQuorum && quorumPriority > rolePriorityMap[role.Name] {
quorumPriority = rolePriorityMap[role.Name]
if role.ParticipatesInQuorum && quorumPriority > rolePriority {
quorumPriority = rolePriority
}
}

Expand All @@ -157,7 +158,7 @@ func (p *realUpdatePlan) buildBestEffortParallelUpdatePlan(rolePriorityMap map[s
instanceList := p.instances
for i, inst := range instanceList {
roleName := getInstanceRoleName(&inst)
if rolePriorityMap[roleName] < quorumPriority {
if getRolePriority(rolePriorityMap, roleName) < quorumPriority {
vertex := &model.ObjectVertex{Obj: &instanceList[i]}
p.dag.AddConnect(preVertex, vertex)
currentVertex = vertex
Expand All @@ -171,7 +172,7 @@ func (p *realUpdatePlan) buildBestEffortParallelUpdatePlan(rolePriorityMap map[s
followerCount := 0
for _, inst := range instanceList {
roleName := getInstanceRoleName(&inst)
if rolePriorityMap[roleName] < leaderPriority {
if getRolePriority(rolePriorityMap, roleName) < leaderPriority {
followerCount++
}
}
Expand Down
112 changes: 112 additions & 0 deletions pkg/controller/instanceset2/update_plan_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
Copyright (C) 2022-2026 ApeCloud Co., Ltd

This file is part of KubeBlocks project

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Affero General Public License for more details.

You should have received a copy of the GNU Affero General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package instanceset2

import (
"reflect"
"sort"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"

workloads "github.com/apecloud/kubeblocks/apis/workloads/v1"
"github.com/apecloud/kubeblocks/pkg/controller/revisionmap"
)

func TestBestEffortParallelUpdatePlanNormalizesMixedCaseRoles(t *testing.T) {
roles := []workloads.ReplicaRole{
{Name: "Follower", ParticipatesInQuorum: true, UpdatePriority: 1},
{Name: "Leader", ParticipatesInQuorum: true, UpdatePriority: 2},
}
instances := []*workloads.Instance{
newUpdatePlanTestInstance("mysql-0", "Follower", roles),
newUpdatePlanTestInstance("mysql-1", "Follower", roles),
newUpdatePlanTestInstance("mysql-2", "Follower", roles),
newUpdatePlanTestInstance("mysql-3", "Follower", roles),
newUpdatePlanTestInstance("mysql-4", "Leader", roles),
}
updateRevisions := make(map[string]string, len(instances))
for _, inst := range instances {
updateRevisions[inst.Name] = "new-revision"
}
encodedRevisions, err := revisionmap.Encode(updateRevisions)
if err != nil {
t.Fatalf("encode update revisions: %v", err)
}
its := workloads.InstanceSet{
Spec: workloads.InstanceSetSpec{
Roles: roles,
MemberUpdateStrategy: ptr.To(workloads.BestEffortParallelUpdateStrategy),
},
Status: workloads.InstanceSetStatus{UpdateRevisions: encodedRevisions},
}

expectedLayers := [][]string{
{"mysql-2", "mysql-3"},
{"mysql-0", "mysql-1"},
{"mysql-4"},
}
for _, expected := range expectedLayers {
updated, err := newUpdatePlan(its, instances).Execute()
if err != nil {
t.Fatalf("execute update plan: %v", err)
}
if got := sortedInstanceNames(updated); !reflect.DeepEqual(got, expected) {
t.Fatalf("updated instances = %v, want %v", got, expected)
}
for _, updatedInst := range updated {
for _, inst := range instances {
if inst.Name == updatedInst.Name {
inst.Annotations[instanceSetRevisionAnnotationKey] = "new-revision"
break
}
}
}
}
}

func newUpdatePlanTestInstance(name, role string, roles []workloads.ReplicaRole) *workloads.Instance {
return &workloads.Instance{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Generation: 1,
Annotations: map[string]string{instanceSetRevisionAnnotationKey: "old-revision"},
},
Spec: workloads.InstanceSpec{Roles: roles},
Status: workloads.InstanceStatus2{
ObservedGeneration: 1,
UpToDate: true,
Role: role,
Conditions: []metav1.Condition{
{Type: string(workloads.InstanceReady), Status: metav1.ConditionTrue},
},
},
}
}

func sortedInstanceNames(instances []*workloads.Instance) []string {
names := make([]string, 0, len(instances))
for _, inst := range instances {
names = append(names, inst.Name)
}
sort.Strings(names)
return names
}
9 changes: 6 additions & 3 deletions pkg/controller/instanceset2/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ func composeRolePriorityMap(roles []workloads.ReplicaRole) map[string]int {
return rolePriorityMap
}

func getRolePriority(rolePriorityMap map[string]int, roleName string) int {
return rolePriorityMap[strings.ToLower(roleName)]
}

// sortInstances sorts instances by their role priority
// e.g.: unknown -> empty -> learner -> follower1 -> follower2 -> leader, with follower1.Name > follower2.Name
// reverse it if reverse==true
Expand All @@ -61,8 +65,7 @@ func sortInstanceObjects(instances []*workloads.Instance, rolePriorityMap map[st
func sortInstancesByRole[T any](instances []T, instanceAt func(int) *workloads.Instance,
rolePriorityMap map[string]int, reverse bool) {
getRolePriorityFunc := func(i int) int {
role := getInstanceRoleName(instanceAt(i))
return rolePriorityMap[role]
return getRolePriority(rolePriorityMap, getInstanceRoleName(instanceAt(i)))
}
getNameNOrdinalFunc := func(i int) (string, int) {
return parseParentNameAndOrdinal(instanceAt(i).GetName())
Expand All @@ -71,7 +74,7 @@ func sortInstancesByRole[T any](instances []T, instanceAt func(int) *workloads.I
}

func getInstanceRoleName(inst *workloads.Instance) string {
return inst.Status.Role
return strings.ToLower(inst.Status.Role)
Comment thread
leon-ape marked this conversation as resolved.
}

func composeRoleMap(its workloads.InstanceSet) map[string]workloads.ReplicaRole {
Expand Down
Loading