diff --git a/cmd/otel-allocator/allocation/per_node.go b/cmd/otel-allocator/allocation/per_node.go index a5e2bfa3f8..864b7aee26 100644 --- a/cmd/otel-allocator/allocation/per_node.go +++ b/cmd/otel-allocator/allocation/per_node.go @@ -25,12 +25,14 @@ const perNodeStrategyName = "per-node" var _ Strategy = &perNodeStrategy{} type perNodeStrategy struct { - collectorByNode map[string]*Collector + collectorByNode map[string]*Collector + fallbackStrategy Strategy } -func newPerNodeStrategy() Strategy { +func newPerNodeStrategy(fallbackStrategy Strategy) Strategy { return &perNodeStrategy{ - collectorByNode: make(map[string]*Collector), + collectorByNode: make(map[string]*Collector), + fallbackStrategy: fallbackStrategy, } } @@ -40,6 +42,10 @@ func (s *perNodeStrategy) GetName() string { func (s *perNodeStrategy) GetCollectorForTarget(collectors map[string]*Collector, item *target.Item) (*Collector, error) { targetNodeName := item.GetNodeName() + if targetNodeName == "" { + return s.fallbackStrategy.GetCollectorForTarget(collectors, item) + } + collector, ok := s.collectorByNode[targetNodeName] if !ok { return nil, fmt.Errorf("could not find collector for node %s", targetNodeName) @@ -54,4 +60,5 @@ func (s *perNodeStrategy) SetCollectors(collectors map[string]*Collector) { s.collectorByNode[collector.NodeName] = collector } } + s.fallbackStrategy.SetCollectors(collectors) } diff --git a/cmd/otel-allocator/allocation/per_node_test.go b/cmd/otel-allocator/allocation/per_node_test.go index ebbe3f31e6..3db6644350 100644 --- a/cmd/otel-allocator/allocation/per_node_test.go +++ b/cmd/otel-allocator/allocation/per_node_test.go @@ -26,6 +26,15 @@ import ( var loggerPerNode = logf.Log.WithName("unit-tests") +func GetTargetsWithNodeName(targets []*target.Item) (targetsWithNodeName []*target.Item) { + for _, item := range targets { + if item.GetNodeName() != "" { + targetsWithNodeName = append(targetsWithNodeName, item) + } + } + return targetsWithNodeName +} + // Tests that two targets with the same target url and job name but different label set are both added. func TestAllocationPerNode(t *testing.T) { // prepare allocator with initial targets and collectors @@ -83,13 +92,18 @@ func TestAllocationPerNode(t *testing.T) { // only the first two targets should be allocated itemsForCollector := s.GetTargetsForCollectorAndJob(actualItem.CollectorName, actualItem.JobName) - // first two should be assigned one to each collector; if third target, should not be assigned + // first two should be assigned one to each collector; if third target, it should be assigned + // according to the fallback strategy which may assign it to the otherwise empty collector or + // one of the others, depending on the strategy and collector loop order if targetHash == thirdTarget.Hash() { - assert.Len(t, itemsForCollector, 0) + assert.Empty(t, item.GetNodeName()) + assert.LessOrEqual(t, len(itemsForCollector), 2) continue } - assert.Len(t, itemsForCollector, 1) - assert.Equal(t, actualItem, itemsForCollector[0]) + + // Only check targets that have been assigned using the per-node (not fallback) strategy here + assert.Len(t, GetTargetsWithNodeName(itemsForCollector), 1) + assert.Equal(t, actualItem, GetTargetsWithNodeName(itemsForCollector)[0]) } } diff --git a/cmd/otel-allocator/allocation/strategy.go b/cmd/otel-allocator/allocation/strategy.go index 29ae7fd99a..c9d9e64a3f 100644 --- a/cmd/otel-allocator/allocation/strategy.go +++ b/cmd/otel-allocator/allocation/strategy.go @@ -149,7 +149,7 @@ func init() { panic(err) } err = Register(perNodeStrategyName, func(log logr.Logger, opts ...AllocationOption) Allocator { - return newAllocator(log, newPerNodeStrategy(), opts...) + return newAllocator(log, newPerNodeStrategy(newleastWeightedStrategy()), opts...) }) if err != nil { panic(err)