Merge pull request #4145 from halfrost/fix-modeling-issue

Fix: karmada-controller-manager: panic: runtime error: index out of range

pkg/controllers/status/cluster_status_controller.go

@@ -618,8 +618,8 @@ func getAllocatableModelings(cluster *clusterv1alpha1.Cluster, nodes []*corev1.N
 		modelingSummary.AddToResourceSummary(modeling.NewClusterResourceNode(nodeAvailable))
 	}
 
-	m := make([]clusterv1alpha1.AllocatableModeling, len(modelingSummary))
+	m := make([]clusterv1alpha1.AllocatableModeling, len(modelingSummary.RMs))
-	for index, resourceModel := range modelingSummary {
+	for index, resourceModel := range modelingSummary.RMs {
 		m[index].Grade = cluster.Spec.ResourceModels[index].Grade
 		m[index].Count = resourceModel.Quantity
 	}

pkg/modeling/modeling.go

@@ -16,12 +16,14 @@ import (
 
 var (
 	mu                  sync.Mutex
-	modelSortings       [][]resource.Quantity
 	defaultModelSorting []clusterapis.ResourceName
 )
 
 // ResourceSummary records the list of resourceModels
-type ResourceSummary []resourceModels
+type ResourceSummary struct {
+	RMs           []resourceModels
+	modelSortings [][]resource.Quantity
+}
 
 // resourceModels records the number of each allocatable resource models.
 type resourceModels struct {
@@ -62,10 +64,10 @@ type ClusterResourceNode struct {
 type ResourceList map[clusterapis.ResourceName]resource.Quantity
 
 // InitSummary is the init function of modeling data structure
-func InitSummary(resourceModels []clusterapis.ResourceModel) (ResourceSummary, error) {
+func InitSummary(resourceModel []clusterapis.ResourceModel) (ResourceSummary, error) {
 	var rsName []clusterapis.ResourceName
 	var rsList []ResourceList
-	for _, rm := range resourceModels {
+	for _, rm := range resourceModel {
 		tmp := map[clusterapis.ResourceName]resource.Quantity{}
 		for _, rmItem := range rm.Ranges {
 			if len(rsName) != len(rm.Ranges) {
@@ -77,21 +79,21 @@ func InitSummary(resourceModels []clusterapis.ResourceModel) (ResourceSummary, e
 	}
 
 	if len(rsName) != 0 && len(rsList) != 0 && (len(rsName) != len(rsList[0])) {
-		return nil, errors.New("the number of resourceName is not equal the number of resourceList")
+		return ResourceSummary{}, errors.New("the number of resourceName is not equal the number of resourceList")
 	}
-	var rs ResourceSummary
+
 	if len(rsName) != 0 {
 		defaultModelSorting = rsName
 	}
-	rs = make(ResourceSummary, len(rsList))
+	rms := make([]resourceModels, len(rsList))
 	// generate a sorted array by first priority of ResourceName
-	modelSortings = make([][]resource.Quantity, len(rsName))
+	modelSortings := make([][]resource.Quantity, len(rsName))
 	for index := 0; index < len(rsList); index++ {
 		for i, name := range rsName {
 			modelSortings[i] = append(modelSortings[i], rsList[index][name])
 		}
 	}
-	return rs, nil
+	return ResourceSummary{RMs: rms, modelSortings: modelSortings}, nil
 }
 
 // NewClusterResourceNode create new cluster resource node
@@ -105,7 +107,7 @@ func NewClusterResourceNode(resourceList corev1.ResourceList) ClusterResourceNod
 func (rs *ResourceSummary) getIndex(crn ClusterResourceNode) int {
 	index := math.MaxInt
 	for i, m := range defaultModelSorting {
-		tmpIndex := searchLastLessElement(modelSortings[i], crn.resourceList[m])
+		tmpIndex := searchLastLessElement(rs.modelSortings[i], crn.resourceList[m])
 		if tmpIndex < index {
 			index = tmpIndex
 		}
@@ -166,7 +168,7 @@ func (rs *ResourceSummary) AddToResourceSummary(crn ClusterResourceNode) {
 		klog.Error("ClusterResource can not add to resource summary: index is invalid.")
 		return
 	}
-	modeling := &(*rs)[index]
+	modeling := &(*rs).RMs[index]
 	if rs.GetNodeNumFromModel(modeling) <= 5 {
 		root := modeling.linkedlist
 		if root == nil {
@@ -269,67 +271,3 @@ func (rs *ResourceSummary) GetNodeNumFromModel(model *resourceModels) int {
 	}
 	return 0
 }
-
-// DeleteFromResourceSummary deletes resource node into modeling summary
-func (rs *ResourceSummary) DeleteFromResourceSummary(crn ClusterResourceNode) error {
-	index := rs.getIndex(crn)
-	if index == -1 {
-		return errors.New("ClusterResource can not delete the resource summary: index is invalid")
-	}
-	modeling := &(*rs)[index]
-	if rs.GetNodeNumFromModel(modeling) >= 6 {
-		root := modeling.redblackTree
-		tmpNode := root.GetNode(crn)
-		if tmpNode != nil {
-			node := tmpNode.Key.(ClusterResourceNode)
-			safeChangeNum(&node.quantity, -crn.quantity)
-			tmpNode.Key = node
-			if node.quantity == 0 {
-				root.Remove(tmpNode)
-			}
-		} else {
-			return errors.New("delete fail: node no found in redblack tree")
-		}
-		modeling.redblackTree = root
-	} else {
-		root, tree := modeling.linkedlist, modeling.redblackTree
-		if root == nil && tree != nil {
-			root = rbtConvertToLl(tree)
-		}
-		if root == nil && tree == nil {
-			return errors.New("delete fail: node no found in linked list")
-		}
-		found := false
-		// traverse linkedlist to remove quantity of recourse modeling
-		for element := root.Front(); element != nil; element = element.Next() {
-			if clusterResourceNodeComparator(element.Value, crn) == 0 {
-				tmpCrn := element.Value.(ClusterResourceNode)
-				safeChangeNum(&tmpCrn.quantity, -crn.quantity)
-				element.Value = tmpCrn
-				if tmpCrn.quantity == 0 {
-					root.Remove(element)
-				}
-				found = true
-			}
-			if found {
-				break
-			}
-		}
-		if !found {
-			return errors.New("delete fail: node no found in linkedlist")
-		}
-		modeling.linkedlist = root
-	}
-	safeChangeNum(&modeling.Quantity, -crn.quantity)
-	return nil
-}
-
-// UpdateInResourceSummary update resource node into modeling summary
-func (rs *ResourceSummary) UpdateInResourceSummary(oldNode, newNode ClusterResourceNode) error {
-	rs.AddToResourceSummary(newNode)
-	err := rs.DeleteFromResourceSummary(oldNode)
-	if err != nil {
-		return errors.New("delete fail: node no found in linked list")
-	}
-	return nil
-}

pkg/modeling/modeling_test.go

@@ -48,7 +48,7 @@ func TestInitSummary(t *testing.T) {
 	}
 
 	rs, err := InitSummary(rms)
-	if actualValue := len(rs); actualValue != 2 {
+	if actualValue := len(rs.RMs); actualValue != 2 {
 		t.Errorf("Got %v expected %v", actualValue, 2)
 	}
 
@@ -87,7 +87,7 @@ func TestInitSummaryError(t *testing.T) {
 	}
 
 	rs, err := InitSummary(rms)
-	if actualValue := len(rs); actualValue != 0 {
+	if actualValue := len(rs.RMs); actualValue != 0 {
 		t.Errorf("Got %v expected %v", actualValue, 0)
 	}
 
@@ -116,7 +116,7 @@ func TestInitSummaryWithOneGrade(t *testing.T) {
 	}
 
 	rs, err := InitSummary(rms)
-	if actualValue := len(rs); actualValue != 1 {
+	if actualValue := len(rs.RMs); actualValue != 1 {
 		t.Errorf("Got %v expected %v", actualValue, 1)
 	}
 
@@ -323,7 +323,7 @@ func TestGetNodeNumFromModel(t *testing.T) {
 	}
 
 	rs, err := InitSummary(rms)
-	if actualValue := len(rs); actualValue != 2 {
+	if actualValue := len(rs.RMs); actualValue != 2 {
 		t.Errorf("Got %v expected %v", actualValue, 2)
 	}
 
@@ -359,8 +359,8 @@ func TestGetNodeNumFromModel(t *testing.T) {
 	rs.AddToResourceSummary(crn2)
 	rs.AddToResourceSummary(crn3)
 
-	for index := range rs {
+	for index := range rs.RMs {
-		num := rs.GetNodeNumFromModel(&rs[index])
+		num := rs.GetNodeNumFromModel(&rs.RMs[index])
 		if index == 0 {
 			if num != 0 {
 				t.Errorf("Got %v expected %v", num, 0)
@@ -622,7 +622,7 @@ func TestAddToResourceSummary(t *testing.T) {
 	}
 
 	rs, err := InitSummary(rms)
-	if actualValue := len(rs); actualValue != 2 {
+	if actualValue := len(rs.RMs); actualValue != 2 {
 		t.Errorf("Got %v expected %v", actualValue, 2)
 	}
 
@@ -658,8 +658,8 @@ func TestAddToResourceSummary(t *testing.T) {
 	rs.AddToResourceSummary(crn2)
 	rs.AddToResourceSummary(crn3)
 
-	for index, v := range rs {
+	for index, v := range rs.RMs {
-		num := rs.GetNodeNumFromModel(&rs[index])
+		num := rs.GetNodeNumFromModel(&rs.RMs[index])
 		if index == 0 && num != 0 {
 			t.Errorf("Got %v expected %v", num, 0)
 		}
@@ -684,239 +684,11 @@ func TestAddToResourceSummary(t *testing.T) {
 
 	rs.AddToResourceSummary(crn4)
 
-	if actualValue := rs[0]; actualValue.Quantity != 2 {
+	if actualValue := rs.RMs[0]; actualValue.Quantity != 2 {
 		t.Errorf("Got %v expected %v", actualValue, 2)
 	}
 
-	if actualValue := rs[0]; rs.GetNodeNumFromModel(&actualValue) != 1 {
+	if actualValue := rs.RMs[0]; rs.GetNodeNumFromModel(&actualValue) != 1 {
 		t.Errorf("Got %v expected %v", actualValue, 1)
 	}
 }
-
-func TestDeleteFromResourceSummary(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 actualValue := len(rs); actualValue != 2 {
-		t.Errorf("Got %v expected %v", actualValue, 2)
-	}
-
-	if err != nil {
-		t.Errorf("Got %v expected %v", err, nil)
-	}
-
-	crn1 := ClusterResourceNode{
-		quantity: 3,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(8, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*3, resource.DecimalSI),
-		},
-	}
-
-	crn2 := ClusterResourceNode{
-		quantity: 1,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*6, resource.DecimalSI),
-		},
-	}
-
-	crn3 := ClusterResourceNode{
-		quantity: 2,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*6, resource.DecimalSI),
-		},
-	}
-
-	rs.AddToResourceSummary(crn1)
-	rs.AddToResourceSummary(crn2)
-	rs.AddToResourceSummary(crn3)
-
-	for index := range rs {
-		num := rs.GetNodeNumFromModel(&rs[index])
-		if index == 0 && num != 0 {
-			t.Errorf("Got %v expected %v", num, 0)
-		}
-		if index == 1 && num != 2 {
-			t.Errorf("Got %v expected %v", num, 2)
-		}
-		if index == 0 && rs[index].Quantity != 0 {
-			t.Errorf("Got %v expected %v", rs[index].Quantity, 0)
-		}
-		if index == 1 && rs[index].Quantity != 6 {
-			t.Errorf("Got %v expected %v", rs[index].Quantity, 6)
-		}
-	}
-
-	crn4 := ClusterResourceNode{
-		quantity: 2,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(19, resource.DecimalSI),
-		},
-	}
-
-	err = rs.DeleteFromResourceSummary(crn4)
-
-	if err == nil {
-		t.Errorf("Got %v expected %v", err, nil)
-	}
-
-	if actualValue := rs[1]; actualValue.Quantity != 6 {
-		t.Errorf("Got %v expected %v", actualValue, 6)
-	}
-
-	if actualValue := rs[0]; rs.GetNodeNumFromModel(&actualValue) != 0 {
-		t.Errorf("Got %v expected %v", actualValue, 0)
-	}
-}
-
-func TestUpdateSummary(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 actualValue := len(rs); actualValue != 2 {
-		t.Errorf("Got %v expected %v", actualValue, 2)
-	}
-
-	if err != nil {
-		t.Errorf("Got %v expected %v", err, nil)
-	}
-
-	crn1 := ClusterResourceNode{
-		quantity: 3,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(8, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*3, resource.DecimalSI),
-		},
-	}
-
-	crn2 := ClusterResourceNode{
-		quantity: 1,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*6, resource.DecimalSI),
-		},
-	}
-
-	crn3 := ClusterResourceNode{
-		quantity: 2,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*6, resource.DecimalSI),
-		},
-	}
-
-	rs.AddToResourceSummary(crn1)
-	rs.AddToResourceSummary(crn2)
-	rs.AddToResourceSummary(crn3)
-
-	for index := range rs {
-		num := rs.GetNodeNumFromModel(&rs[index])
-		if index == 0 && num != 0 {
-			t.Errorf("Got %v expected %v", num, 0)
-		}
-		if index == 1 && num != 2 {
-			t.Errorf("Got %v expected %v", num, 2)
-		}
-		if index == 0 && rs[index].Quantity != 0 {
-			t.Errorf("Got %v expected %v", rs[index].Quantity, 0)
-		}
-		if index == 1 && rs[index].Quantity != 6 {
-			t.Errorf("Got %v expected %v", rs[index].Quantity, 6)
-		}
-	}
-
-	crn2 = ClusterResourceNode{
-		quantity: 1,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(1024*6, resource.DecimalSI),
-		},
-	}
-
-	crn4 := ClusterResourceNode{
-		quantity: 2,
-		resourceList: ResourceList{
-			clusterapis.ResourceCPU:    *resource.NewMilliQuantity(1, resource.DecimalSI),
-			clusterapis.ResourceMemory: *resource.NewQuantity(19, resource.DecimalSI),
-		},
-	}
-
-	err = rs.UpdateInResourceSummary(crn2, crn4)
-
-	if err != nil {
-		t.Errorf("Got %v expected %v", err, nil)
-	}
-
-	if actualValue := rs[1]; actualValue.Quantity != 7 {
-		t.Errorf("Got %v expected %v", actualValue, 7)
-	}
-
-	if actualValue := rs[1]; rs.GetNodeNumFromModel(&actualValue) != 3 {
-		t.Errorf("Got %v expected %v", rs.GetNodeNumFromModel(&actualValue), 3)
-	}
-}