Merge pull request #117249 from tnqn/fix-data-race
Fix a data race in TopologyCache
This commit is contained in:
		| @@ -270,6 +270,9 @@ func (t *TopologyCache) HasPopulatedHints(serviceKey string) bool { | |||||||
| // it is not possible to provide allocations that are below the overload | // it is not possible to provide allocations that are below the overload | ||||||
| // threshold, a nil value will be returned. | // threshold, a nil value will be returned. | ||||||
| func (t *TopologyCache) getAllocations(numEndpoints int) (map[string]Allocation, *EventBuilder) { | func (t *TopologyCache) getAllocations(numEndpoints int) (map[string]Allocation, *EventBuilder) { | ||||||
|  | 	t.lock.Lock() | ||||||
|  | 	defer t.lock.Unlock() | ||||||
|  |  | ||||||
| 	// it is similar to checking !t.sufficientNodeInfo | 	// it is similar to checking !t.sufficientNodeInfo | ||||||
| 	if t.cpuRatiosByZone == nil { | 	if t.cpuRatiosByZone == nil { | ||||||
| 		return nil, &EventBuilder{ | 		return nil, &EventBuilder{ | ||||||
| @@ -293,9 +296,6 @@ func (t *TopologyCache) getAllocations(numEndpoints int) (map[string]Allocation, | |||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	t.lock.Lock() |  | ||||||
| 	defer t.lock.Unlock() |  | ||||||
|  |  | ||||||
| 	remainingMinEndpoints := numEndpoints | 	remainingMinEndpoints := numEndpoints | ||||||
| 	minTotal := 0 | 	minTotal := 0 | ||||||
| 	allocations := map[string]Allocation{} | 	allocations := map[string]Allocation{} | ||||||
|   | |||||||
| @@ -625,6 +625,69 @@ func TestSetNodes(t *testing.T) { | |||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
|  | func TestTopologyCacheRace(t *testing.T) { | ||||||
|  | 	sliceInfo := &SliceInfo{ | ||||||
|  | 		ServiceKey:  "ns/svc", | ||||||
|  | 		AddressType: discovery.AddressTypeIPv4, | ||||||
|  | 		ToCreate: []*discovery.EndpointSlice{{ | ||||||
|  | 			Endpoints: []discovery.Endpoint{{ | ||||||
|  | 				Addresses:  []string{"10.1.2.3"}, | ||||||
|  | 				Zone:       pointer.String("zone-a"), | ||||||
|  | 				Conditions: discovery.EndpointConditions{Ready: pointer.Bool(true)}, | ||||||
|  | 			}, { | ||||||
|  | 				Addresses:  []string{"10.1.2.4"}, | ||||||
|  | 				Zone:       pointer.String("zone-b"), | ||||||
|  | 				Conditions: discovery.EndpointConditions{Ready: pointer.Bool(true)}, | ||||||
|  | 			}}, | ||||||
|  | 		}}} | ||||||
|  | 	type nodeInfo struct { | ||||||
|  | 		zone  string | ||||||
|  | 		cpu   resource.Quantity | ||||||
|  | 		ready v1.ConditionStatus | ||||||
|  | 	} | ||||||
|  | 	nodeInfos := []nodeInfo{ | ||||||
|  | 		{zone: "zone-a", cpu: resource.MustParse("1000m"), ready: v1.ConditionTrue}, | ||||||
|  | 		{zone: "zone-a", cpu: resource.MustParse("1000m"), ready: v1.ConditionTrue}, | ||||||
|  | 		{zone: "zone-a", cpu: resource.MustParse("1000m"), ready: v1.ConditionTrue}, | ||||||
|  | 		{zone: "zone-a", cpu: resource.MustParse("2000m"), ready: v1.ConditionTrue}, | ||||||
|  | 		{zone: "zone-b", cpu: resource.MustParse("3000m"), ready: v1.ConditionTrue}, | ||||||
|  | 		{zone: "zone-b", cpu: resource.MustParse("1500m"), ready: v1.ConditionTrue}, | ||||||
|  | 		{zone: "zone-c", cpu: resource.MustParse("500m"), ready: v1.ConditionTrue}, | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	cache := NewTopologyCache() | ||||||
|  | 	nodes := []*v1.Node{} | ||||||
|  | 	for _, node := range nodeInfos { | ||||||
|  | 		labels := map[string]string{} | ||||||
|  | 		if node.zone != "" { | ||||||
|  | 			labels[v1.LabelTopologyZone] = node.zone | ||||||
|  | 		} | ||||||
|  | 		conditions := []v1.NodeCondition{{ | ||||||
|  | 			Type:   v1.NodeReady, | ||||||
|  | 			Status: node.ready, | ||||||
|  | 		}} | ||||||
|  | 		allocatable := v1.ResourceList{ | ||||||
|  | 			v1.ResourceCPU: node.cpu, | ||||||
|  | 		} | ||||||
|  | 		nodes = append(nodes, &v1.Node{ | ||||||
|  | 			ObjectMeta: metav1.ObjectMeta{ | ||||||
|  | 				Labels: labels, | ||||||
|  | 			}, | ||||||
|  | 			Status: v1.NodeStatus{ | ||||||
|  | 				Allocatable: allocatable, | ||||||
|  | 				Conditions:  conditions, | ||||||
|  | 			}, | ||||||
|  | 		}) | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	go func() { | ||||||
|  | 		cache.SetNodes(nodes) | ||||||
|  | 	}() | ||||||
|  | 	go func() { | ||||||
|  | 		cache.AddHints(sliceInfo) | ||||||
|  | 	}() | ||||||
|  | } | ||||||
|  |  | ||||||
| // Test Helpers | // Test Helpers | ||||||
|  |  | ||||||
| func expectEquivalentSlices(t *testing.T, actualSlices, expectedSlices []*discovery.EndpointSlice) { | func expectEquivalentSlices(t *testing.T, actualSlices, expectedSlices []*discovery.EndpointSlice) { | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Kubernetes Prow Robot
					Kubernetes Prow Robot