Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allocation_fallback_strategy option as fallback strategy for per-node strategy #3482

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
12e1282
Add least-weighted as fallback strategy for per-node strategy
lhp-nemlig Nov 21, 2024
b7649be
Add changelog file
lhp-nemlig Nov 21, 2024
31982b7
Change fallback strategy to consistent-hashing
lhp-nemlig Nov 21, 2024
92d0069
Update changelog
lhp-nemlig Nov 21, 2024
381222c
Fix bad test condition that might pass even if target was not assigned
lhp-nemlig Nov 21, 2024
ee6e856
Make fallback strategy a config option
lhp-nemlig Nov 21, 2024
ef2d720
Update changelog
lhp-nemlig Nov 21, 2024
ea6e956
Merge branch 'main' into add_fallback_strategy_for_per_node_strategy
lhp-nemlig Nov 21, 2024
6f757c3
Add period to test comments
lhp-nemlig Nov 21, 2024
e62f7d2
Merge branch 'main' into add_fallback_strategy_for_per_node_strategy
lhp-nemlig Nov 22, 2024
7711a32
Add feature gate for enabling fallback strategy
lhp-nemlig Nov 22, 2024
e1207cd
Fix featuregate id
lhp-nemlig Nov 22, 2024
7eabd10
Update cmd/otel-allocator/allocation/per_node_test.go
lhp-nemlig Nov 22, 2024
6be618c
Update cmd/otel-allocator/allocation/per_node_test.go
lhp-nemlig Nov 22, 2024
7d32f7e
Update cmd/otel-allocator/allocation/per_node_test.go
lhp-nemlig Nov 22, 2024
0254654
Only add fallbackstrategy if nonempty
lhp-nemlig Nov 22, 2024
a5b8dca
Remove unnecessary comments
lhp-nemlig Nov 22, 2024
d6435e3
Add unit test for fallbackstrategy feature gate
lhp-nemlig Nov 22, 2024
0f878f0
Update changelog
lhp-nemlig Nov 22, 2024
bf5a367
Merge branch 'main' into add_fallback_strategy_for_per_node_strategy
lhp-nemlig Nov 22, 2024
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
13 changes: 10 additions & 3 deletions cmd/otel-allocator/allocation/per_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand All @@ -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)
Expand All @@ -54,4 +60,5 @@ func (s *perNodeStrategy) SetCollectors(collectors map[string]*Collector) {
s.collectorByNode[collector.NodeName] = collector
}
}
s.fallbackStrategy.SetCollectors(collectors)
}
22 changes: 18 additions & 4 deletions cmd/otel-allocator/allocation/per_node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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])
}
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/otel-allocator/allocation/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
swiatekm marked this conversation as resolved.
Show resolved Hide resolved
})
if err != nil {
panic(err)
Expand Down
Loading