Merge pull request #4176 from jwcesign/master

fix: change defaultModelSorting as local variable

pkg/modeling/modeling.go

@@ -4,7 +4,6 @@ import (
 	"container/list"
 	"errors"
 	"math"
-	"sync"
 
 	rbt "github.com/emirpasic/gods/trees/redblacktree"
 	corev1 "k8s.io/api/core/v1"
@@ -14,15 +13,11 @@ import (
 	clusterapis "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
 )
 
-var (
-	mu                  sync.Mutex
-	defaultModelSorting []clusterapis.ResourceName
-)
-
 // ResourceSummary records the list of resourceModels
 type ResourceSummary struct {
-	RMs           []resourceModels
-	modelSortings [][]resource.Quantity
+	RMs                       []resourceModels
+	modelSortings             [][]resource.Quantity
+	modelSortingResourceNames []clusterapis.ResourceName
 }
 
 // resourceModels records the number of each allocatable resource models.
@@ -82,9 +77,6 @@ func InitSummary(resourceModel []clusterapis.ResourceModel) (ResourceSummary, er
 		return ResourceSummary{}, errors.New("the number of resourceName is not equal the number of resourceList")
 	}
 
-	if len(rsName) != 0 {
-		defaultModelSorting = rsName
-	}
 	rms := make([]resourceModels, len(rsList))
 	// generate a sorted array by first priority of ResourceName
 	modelSortings := make([][]resource.Quantity, len(rsName))
@@ -93,7 +85,7 @@ func InitSummary(resourceModel []clusterapis.ResourceModel) (ResourceSummary, er
 			modelSortings[i] = append(modelSortings[i], rsList[index][name])
 		}
 	}
-	return ResourceSummary{RMs: rms, modelSortings: modelSortings}, nil
+	return ResourceSummary{RMs: rms, modelSortings: modelSortings, modelSortingResourceNames: rsName}, nil
 }
 
 // NewClusterResourceNode create new cluster resource node
@@ -106,7 +98,7 @@ func NewClusterResourceNode(resourceList corev1.ResourceList) ClusterResourceNod
 
 func (rs *ResourceSummary) getIndex(crn ClusterResourceNode) int {
 	index := math.MaxInt
-	for i, m := range defaultModelSorting {
+	for i, m := range rs.modelSortingResourceNames {
 		tmpIndex := searchLastLessElement(rs.modelSortings[i], crn.resourceList[m])
 		if tmpIndex < index {
 			index = tmpIndex
@@ -140,11 +132,11 @@ func searchLastLessElement(nums []resource.Quantity, target resource.Quantity) i
 }
 
 // clusterResourceNodeComparator provides a fast comparison on clusterResourceNodes
-func clusterResourceNodeComparator(a, b interface{}) int {
+func (rs *ResourceSummary) clusterResourceNodeComparator(a, b interface{}) int {
 	s1 := a.(ClusterResourceNode)
 	s2 := b.(ClusterResourceNode)
-	for index := 0; index < len(defaultModelSorting); index++ {
-		tmp1, tmp2 := s1.resourceList[defaultModelSorting[index]], s2.resourceList[defaultModelSorting[index]]
+	for index := 0; index < len(rs.modelSortingResourceNames); index++ {
+		tmp1, tmp2 := s1.resourceList[rs.modelSortingResourceNames[index]], s2.resourceList[rs.modelSortingResourceNames[index]]
 
 		diff := tmp1.Cmp(tmp2)
 		if diff != 0 {
@@ -154,13 +146,6 @@ func clusterResourceNodeComparator(a, b interface{}) int {
 	return 0
 }
 
-func safeChangeNum(num *int, change int) {
-	mu.Lock()
-	defer mu.Unlock()
-
-	*num += change
-}
-
 // AddToResourceSummary add resource node into modeling summary
 func (rs *ResourceSummary) AddToResourceSummary(crn ClusterResourceNode) {
 	index := rs.getIndex(crn)
@@ -177,11 +162,11 @@ func (rs *ResourceSummary) AddToResourceSummary(crn ClusterResourceNode) {
 		found := false
 		// traverse linkedlist to add quantity of recourse modeling
 		for element := root.Front(); element != nil; element = element.Next() {
-			switch clusterResourceNodeComparator(element.Value, crn) {
+			switch rs.clusterResourceNodeComparator(element.Value, crn) {
 			case 0:
 				{
 					tmpCrn := element.Value.(ClusterResourceNode)
-					safeChangeNum(&tmpCrn.quantity, crn.quantity)
+					tmpCrn.quantity += crn.quantity
 					element.Value = tmpCrn
 					found = true
 					break
@@ -208,24 +193,24 @@ func (rs *ResourceSummary) AddToResourceSummary(crn ClusterResourceNode) {
 	} else {
 		root := modeling.redblackTree
 		if root == nil {
-			root = llConvertToRbt(modeling.linkedlist)
+			root = rs.llConvertToRbt(modeling.linkedlist)
 			modeling.linkedlist = nil
 		}
 		tmpNode := root.GetNode(crn)
 		if tmpNode != nil {
 			node := tmpNode.Key.(ClusterResourceNode)
-			safeChangeNum(&node.quantity, crn.quantity)
+			node.quantity += crn.quantity
 			tmpNode.Key = node
 		} else {
 			root.Put(crn, crn.quantity)
 		}
 		modeling.redblackTree = root
 	}
-	safeChangeNum(&modeling.Quantity, crn.quantity)
+	modeling.Quantity += crn.quantity
 }
 
-func llConvertToRbt(list *list.List) *rbt.Tree {
-	root := rbt.NewWith(clusterResourceNodeComparator)
+func (rs *ResourceSummary) llConvertToRbt(list *list.List) *rbt.Tree {
+	root := rbt.NewWith(rs.clusterResourceNodeComparator)
 	for element := list.Front(); element != nil; element = element.Next() {
 		tmpCrn := element.Value.(ClusterResourceNode)
 		root.Put(tmpCrn, tmpCrn.quantity)
@@ -233,14 +218,6 @@ func llConvertToRbt(list *list.List) *rbt.Tree {
 	return root
 }
 
-func rbtConvertToLl(rbt *rbt.Tree) *list.List {
-	root := list.New()
-	for _, v := range rbt.Keys() {
-		root.PushBack(v)
-	}
-	return root
-}
-
 // ConvertToResourceList is convert from corev1.ResourceList to ResourceList
 func ConvertToResourceList(rsList corev1.ResourceList) ResourceList {
 	resourceList := ResourceList{}

pkg/modeling/modeling_test.go

@@ -197,7 +197,6 @@ func TestGetIndex(t *testing.T) {
 	}
 
 	rs, err := InitSummary(rms)
-
 	if err != nil {
 		t.Errorf("Got %v expected %v", err, nil)
 	}
@@ -230,6 +229,44 @@ func TestGetIndex(t *testing.T) {
 }
 
 func TestClusterResourceNodeComparator(t *testing.T) {
+	rms := []clusterapis.ResourceModel{
+		{
+			Grade: 0,
+			Ranges: []clusterapis.ResourceModelRange{
+				{
+					Name: clusterapis.ResourceCPU,
+					Min:  *resource.NewMilliQuantity(0, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1, resource.DecimalSI),
+				},
+				{
+					Name: clusterapis.ResourceMemory,
+					Min:  *resource.NewMilliQuantity(0, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1024, resource.DecimalSI),
+				},
+			},
+		},
+		{
+			Grade: 1,
+			Ranges: []clusterapis.ResourceModelRange{
+				{
+					Name: clusterapis.ResourceCPU,
+					Min:  *resource.NewMilliQuantity(1, resource.DecimalSI),
+					Max:  *resource.NewQuantity(2, resource.DecimalSI),
+				},
+				{
+					Name: clusterapis.ResourceMemory,
+					Min:  *resource.NewMilliQuantity(1024, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1024*2, resource.DecimalSI),
+				},
+			},
+		},
+	}
+
+	rs, err := InitSummary(rms)
+	if err != nil {
+		t.Errorf("Got %v expected %v", err, nil)
+	}
+
 	crn1 := ClusterResourceNode{
 		quantity: 10,
 		resourceList: ResourceList{
@@ -245,7 +282,7 @@ func TestClusterResourceNodeComparator(t *testing.T) {
 			clusterapis.ResourceMemory: *resource.NewQuantity(1024, resource.DecimalSI),
 		},
 	}
-	if res := clusterResourceNodeComparator(crn1, crn2); res != 1 {
+	if res := rs.clusterResourceNodeComparator(crn1, crn2); res != 1 {
 		t.Errorf("Got %v expected %v", res, 1)
 	}
 
@@ -264,7 +301,7 @@ func TestClusterResourceNodeComparator(t *testing.T) {
 			clusterapis.ResourceMemory: *resource.NewQuantity(1024, resource.DecimalSI),
 		},
 	}
-	if res := clusterResourceNodeComparator(crn1, crn2); res != 0 {
+	if res := rs.clusterResourceNodeComparator(crn1, crn2); res != 0 {
 		t.Errorf("Got %v expected %v", res, 0)
 	}
 
@@ -283,7 +320,7 @@ func TestClusterResourceNodeComparator(t *testing.T) {
 			clusterapis.ResourceMemory: *resource.NewQuantity(1024*10, resource.DecimalSI),
 		},
 	}
-	if res := clusterResourceNodeComparator(crn1, crn2); res != -1 {
+	if res := rs.clusterResourceNodeComparator(crn1, crn2); res != -1 {
 		t.Errorf("Got %v expected %v", res, -1)
 	}
 }
@@ -388,6 +425,44 @@ func TestConvertToResourceList(t *testing.T) {
 }
 
 func TestLlConvertToRbt(t *testing.T) {
+	rms := []clusterapis.ResourceModel{
+		{
+			Grade: 0,
+			Ranges: []clusterapis.ResourceModelRange{
+				{
+					Name: clusterapis.ResourceCPU,
+					Min:  *resource.NewMilliQuantity(0, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1, resource.DecimalSI),
+				},
+				{
+					Name: clusterapis.ResourceMemory,
+					Min:  *resource.NewMilliQuantity(0, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1024, resource.DecimalSI),
+				},
+			},
+		},
+		{
+			Grade: 1,
+			Ranges: []clusterapis.ResourceModelRange{
+				{
+					Name: clusterapis.ResourceCPU,
+					Min:  *resource.NewMilliQuantity(1, resource.DecimalSI),
+					Max:  *resource.NewQuantity(2, resource.DecimalSI),
+				},
+				{
+					Name: clusterapis.ResourceMemory,
+					Min:  *resource.NewMilliQuantity(1024, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1024*2, resource.DecimalSI),
+				},
+			},
+		},
+	}
+
+	rs, err := InitSummary(rms)
+	if err != nil {
+		t.Errorf("Got %v expected %v", err, nil)
+	}
+
 	crn1 := ClusterResourceNode{
 		quantity: 6,
 		resourceList: ResourceList{
@@ -443,7 +518,7 @@ func TestLlConvertToRbt(t *testing.T) {
 	mylist.PushBack(crn2)
 	mylist.PushBack(crn4)
 
-	rbt := llConvertToRbt(mylist)
+	rbt := rs.llConvertToRbt(mylist)
 	fmt.Println(rbt)
 	if actualValue := rbt.Size(); actualValue != 6 {
 		t.Errorf("Got %v expected %v", actualValue, 6)
@@ -469,54 +544,45 @@ func TestLlConvertToRbt(t *testing.T) {
 }
 
 func TestRbtConvertToLl(t *testing.T) {
-	crn1 := ClusterResourceNode{
-		quantity: 6,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(2, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*7, resource.DecimalSI),
+	rms := []clusterapis.ResourceModel{
+		{
+			Grade: 0,
+			Ranges: []clusterapis.ResourceModelRange{
+				{
+					Name: clusterapis.ResourceCPU,
+					Min:  *resource.NewMilliQuantity(0, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1, resource.DecimalSI),
+				},
+				{
+					Name: clusterapis.ResourceMemory,
+					Min:  *resource.NewMilliQuantity(0, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1024, resource.DecimalSI),
+				},
+			},
+		},
+		{
+			Grade: 1,
+			Ranges: []clusterapis.ResourceModelRange{
+				{
+					Name: clusterapis.ResourceCPU,
+					Min:  *resource.NewMilliQuantity(1, resource.DecimalSI),
+					Max:  *resource.NewQuantity(2, resource.DecimalSI),
+				},
+				{
+					Name: clusterapis.ResourceMemory,
+					Min:  *resource.NewMilliQuantity(1024, resource.DecimalSI),
+					Max:  *resource.NewQuantity(1024*2, resource.DecimalSI),
+				},
+			},
 		},
 	}
 
-	crn2 := ClusterResourceNode{
-		quantity: 5,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(6, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*3, resource.DecimalSI),
-		},
+	rs, err := InitSummary(rms)
+	if err != nil {
+		t.Errorf("Got %v expected %v", err, nil)
 	}
 
-	crn3 := ClusterResourceNode{
-		quantity: 4,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(5, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*8, resource.DecimalSI),
-		},
-	}
-
-	crn4 := ClusterResourceNode{
-		quantity: 3,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(8, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*3, resource.DecimalSI),
-		},
-	}
-
-	crn5 := ClusterResourceNode{
-		quantity: 2,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*6, resource.DecimalSI),
-		},
-	}
-
-	crn6 := ClusterResourceNode{
-		quantity: 1,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(2, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*12, resource.DecimalSI),
-		},
-	}
-	tree := redblacktree.NewWith(clusterResourceNodeComparator)
+	tree := redblacktree.NewWith(rs.clusterResourceNodeComparator)
 
 	if actualValue := tree.Size(); actualValue != 0 {
 		t.Errorf("Got %v expected %v", actualValue, 0)
@@ -525,66 +591,6 @@ func TestRbtConvertToLl(t *testing.T) {
 	if actualValue := tree.GetNode(2).Size(); actualValue != 0 {
 		t.Errorf("Got %v expected %v", actualValue, 0)
 	}
-
-	tree.Put(crn2, crn2.quantity)
-	tree.Put(crn1, crn1.quantity)
-	tree.Put(crn6, crn6.quantity)
-	tree.Put(crn3, crn3.quantity)
-	tree.Put(crn5, crn5.quantity)
-	tree.Put(crn4, crn4.quantity)
-
-	ll := rbtConvertToLl(tree)
-	fmt.Println(ll)
-
-	for element := ll.Front(); element != nil; element = element.Next() {
-		fmt.Println(element.Value)
-	}
-
-	actualValue := ll.Front()
-	node := actualValue.Value.(ClusterResourceNode)
-	if quantity := node.quantity; quantity != 2 {
-		t.Errorf("Got %v expected %v", actualValue, 2)
-	}
-	ll.Remove(actualValue)
-
-	actualValue = ll.Front()
-	node = actualValue.Value.(ClusterResourceNode)
-	if quantity := node.quantity; quantity != 6 {
-		t.Errorf("Got %v expected %v", actualValue, 6)
-	}
-	ll.Remove(actualValue)
-
-	actualValue = ll.Front()
-	node = actualValue.Value.(ClusterResourceNode)
-	if quantity := node.quantity; quantity != 1 {
-		t.Errorf("Got %v expected %v", actualValue, 1)
-	}
-	ll.Remove(actualValue)
-
-	actualValue = ll.Front()
-	node = actualValue.Value.(ClusterResourceNode)
-	if quantity := node.quantity; quantity != 4 {
-		t.Errorf("Got %v expected %v", actualValue, 4)
-	}
-	ll.Remove(actualValue)
-
-	actualValue = ll.Front()
-	node = actualValue.Value.(ClusterResourceNode)
-	if quantity := node.quantity; quantity != 5 {
-		t.Errorf("Got %v expected %v", actualValue, 5)
-	}
-	ll.Remove(actualValue)
-
-	actualValue = ll.Front()
-	node = actualValue.Value.(ClusterResourceNode)
-	if quantity := node.quantity; quantity != 3 {
-		t.Errorf("Got %v expected %v", actualValue, 3)
-	}
-	ll.Remove(actualValue)
-
-	if actualValue := ll.Len(); actualValue != 0 {
-		t.Errorf("Got %v expected %v", actualValue, 0)
-	}
 }
 
 func TestAddToResourceSummary(t *testing.T) {