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

[target-allocator] Introduce "per node" allocation strategy to target allocator #2430

Merged
merged 38 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
6defa78
Add node name to Collector struct
matej-g Dec 5, 2023
318fd6c
Add per node allocation algorithm
matej-g Dec 5, 2023
d298b69
More docs
matej-g Dec 7, 2023
886a80c
Add tests
matej-g Dec 7, 2023
23089e3
Update APIs and docs with new strategy
matej-g Dec 7, 2023
7f82f69
Add changelog
matej-g Dec 8, 2023
959c3c5
Fix lint
matej-g Dec 8, 2023
082e678
Add more labels to match node name
matej-g Dec 8, 2023
2d29ae6
Better handling of unassignable targets / jobs
matej-g Dec 18, 2023
c08f560
Adjust webhook validation
matej-g Dec 18, 2023
8c0f9e6
Add E2E test; put TA E2E tests into separate target
matej-g Dec 18, 2023
5ad0079
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Dec 18, 2023
acf7644
Format; fix mistakenly changed test image
matej-g Dec 18, 2023
57d23bd
Use node name instead of collector name for collector map
matej-g Dec 19, 2023
0e09fc8
Add TA E2E tests to GH action
matej-g Dec 19, 2023
0c8239e
Merge branch 'main' into per-node-strategy
matej-g Dec 19, 2023
9c6c6e6
Bump kuttl timeout
matej-g Dec 19, 2023
6d08c78
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Dec 20, 2023
94b336e
Validate mode and strategy
matej-g Jan 4, 2024
023ce4b
Handle cases of missing node name when (re)adding collector
matej-g Jan 4, 2024
113f6bb
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Jan 8, 2024
5976307
Adjust test cases after node name fix
matej-g Jan 8, 2024
fe5e28d
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Jan 9, 2024
3ac78c7
Adjust E2E tests
matej-g Jan 9, 2024
85c66e5
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Jan 9, 2024
0c3fbdc
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Jan 15, 2024
0b3363e
Put TA config validation into separate method
matej-g Jan 15, 2024
02ee921
Adjust logging and add metric for jobs with unassigned targets
matej-g Jan 15, 2024
d2d46c9
Merge branch 'main' into per-node-strategy
matej-g Jan 15, 2024
485d9d6
Merge branch 'main' into per-node-strategy
matej-g Jan 15, 2024
b8befdf
Address feedback
matej-g Jan 18, 2024
f7eeb5e
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Jan 18, 2024
253b340
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Feb 1, 2024
9075a84
Simplify flow according to feedback; correct docs
matej-g Feb 6, 2024
339ed5c
Add more unit tests for edge cases
matej-g Feb 6, 2024
7defcdc
Merge remote-tracking branch 'origin/main' into per-node-strategy
matej-g Feb 6, 2024
5d31f67
Fix test case assert file
matej-g Feb 6, 2024
4f195d5
Merge branch 'main' into per-node-strategy
matej-g Feb 8, 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
Prev Previous commit
Next Next commit
Use node name instead of collector name for collector map
Signed-off-by: Matej Gera <matejgera@gmail.com>
  • Loading branch information
matej-g committed Dec 19, 2023
commit 57d23bdc9b54c6d7ffaada70c56cc6148755c008
2 changes: 1 addition & 1 deletion cmd/otel-allocator/allocation/allocatortest.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func MakeNCollectors(n int, startingIndex int) map[string]*Collector {
toReturn[collector] = &Collector{
Name: collector,
NumTargets: 0,
Node: fmt.Sprintf("node-%d", i),
NodeName: fmt.Sprintf("node-%d", i),
}
}
return toReturn
Expand Down
2 changes: 1 addition & 1 deletion cmd/otel-allocator/allocation/consistent_hashing.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (c *consistentHashingAllocator) handleCollectors(diff diff.Changes[*Collect
}
// Insert the new collectors
for _, i := range diff.Additions() {
c.collectors[i.Name] = NewCollector(i.Name, i.Node)
c.collectors[i.Name] = NewCollector(i.Name, i.NodeName)
c.consistentHasher.Add(c.collectors[i.Name])
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/otel-allocator/allocation/least_weighted.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func (allocator *leastWeightedAllocator) handleCollectors(diff diff.Changes[*Col
}
// Insert the new collectors
for _, i := range diff.Additions() {
allocator.collectors[i.Name] = NewCollector(i.Name, i.Node)
allocator.collectors[i.Name] = NewCollector(i.Name, i.NodeName)
}
if allocateTargets {
for _, item := range allocator.targetItems {
Expand Down
43 changes: 6 additions & 37 deletions cmd/otel-allocator/allocation/per_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/go-logr/logr"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/common/model"
)

var _ Allocator = &perNodeAllocator{}
Expand All @@ -38,7 +37,7 @@ const perNodeStrategyName = "per-node"
type perNodeAllocator struct {
// m protects collectors and targetItems for concurrent use.
m sync.RWMutex
// collectors is a map from a Collector's name to a Collector instance
// collectors is a map from a Collector's node name to a Collector instance
collectors map[string]*Collector
jaronoff97 marked this conversation as resolved.
Show resolved Hide resolved
// targetItems is a map from a target item's hash to the target items allocated state
targetItems map[string]*target.Item
Expand All @@ -51,15 +50,6 @@ type perNodeAllocator struct {
filter Filter
}

// nodeLabels are labels that are used to identify the node on which the given
// target is residing. To learn more about these labels, please refer to:
// https://prometheus.io/docs/prometheus/latest/configuration/configuration/#kubernetes_sd_config
var nodeLabels = []model.LabelName{
"__meta_kubernetes_pod_node_name",
"__meta_kubernetes_node_name",
"__meta_kubernetes_endpoint_node_name",
}

// SetCollectors sets the set of collectors with key=collectorName, value=Collector object.
// This method is called when Collectors are added or removed.
func (allocator *perNodeAllocator) SetCollectors(collectors map[string]*Collector) {
Expand Down Expand Up @@ -87,14 +77,14 @@ func (allocator *perNodeAllocator) SetCollectors(collectors map[string]*Collecto
func (allocator *perNodeAllocator) handleCollectors(diff diff.Changes[*Collector]) {
// Clear removed collectors
for _, k := range diff.Removals() {
delete(allocator.collectors, k.Name)
delete(allocator.collectors, k.NodeName)
delete(allocator.targetItemsPerJobPerCollector, k.Name)
TargetsPerCollector.WithLabelValues(k.Name, perNodeStrategyName).Set(0)
}

// Insert the new collectors
for _, i := range diff.Additions() {
allocator.collectors[i.Name] = NewCollector(i.Name, i.Node)
allocator.collectors[i.NodeName] = NewCollector(i.Name, i.NodeName)
}
}

Expand Down Expand Up @@ -166,7 +156,7 @@ func (allocator *perNodeAllocator) handleTargets(diff diff.Changes[*target.Item]
for k, item := range allocator.targetItems {
// if the current item is in the removals list
if _, ok := diff.Removals()[k]; ok {
c, ok := allocator.collectors[item.CollectorName]
c, ok := allocator.collectors[item.GetNodeName()]
if !ok {
continue
}
Expand Down Expand Up @@ -214,8 +204,8 @@ func (allocator *perNodeAllocator) handleTargets(diff diff.Changes[*target.Item]
// target items setting.
func (allocator *perNodeAllocator) addTargetToTargetItems(tg *target.Item) bool {
allocator.targetItems[tg.Hash()] = tg
chosenCollector := allocator.findCollector(tg.Labels)
if chosenCollector == nil {
chosenCollector, ok := allocator.collectors[tg.GetNodeName()]
if !ok {
allocator.log.V(2).Info("Couldn't find a collector for the target item", "item", tg, "collectors", allocator.collectors)
return false
}
Expand All @@ -226,27 +216,6 @@ func (allocator *perNodeAllocator) addTargetToTargetItems(tg *target.Item) bool
return true
}

// findCollector finds the collector that matches the node of the target, on the basis of the
// pod node label.
// This method is called from within SetTargets and SetCollectors, whose caller
// acquires the needed lock. This method assumes there are is at least 1 collector set.
func (allocator *perNodeAllocator) findCollector(labels model.LabelSet) *Collector {
var col *Collector
for _, v := range allocator.collectors {
// Try to match against a node label.
for _, l := range nodeLabels {
if nodeNameLabelValue, ok := labels[l]; ok {
if v.Node == string(nodeNameLabelValue) {
col = v
break
}
}
}
}

return col
}

// addCollectorTargetItemMapping keeps track of which collector has which jobs and targets
// this allows the allocator to respond without any extra allocations to http calls. The caller of this method
// has to acquire a lock.
Expand Down
4 changes: 2 additions & 2 deletions cmd/otel-allocator/allocation/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ var _ consistent.Member = Collector{}
// This struct can be extended with information like annotations and labels in the future.
type Collector struct {
Name string
Node string
NodeName string
NumTargets int
}

Expand All @@ -119,7 +119,7 @@ func (c Collector) String() string {
}

func NewCollector(name, node string) *Collector {
return &Collector{Name: name, Node: node}
return &Collector{Name: name, NodeName: node}
}

func init() {
Expand Down
19 changes: 19 additions & 0 deletions cmd/otel-allocator/target/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ import (
"github.com/prometheus/common/model"
)

// nodeLabels are labels that are used to identify the node on which the given
// target is residing. To learn more about these labels, please refer to:
// https://prometheus.io/docs/prometheus/latest/configuration/configuration/#kubernetes_sd_config
var nodeLabels = []model.LabelName{
"__meta_kubernetes_pod_node_name",
"__meta_kubernetes_node_name",
"__meta_kubernetes_endpoint_node_name",
}

// LinkJSON This package contains common structs and methods that relate to scrape targets.
type LinkJSON struct {
Link string `json:"_link"`
Expand All @@ -39,6 +48,16 @@ func (t *Item) Hash() string {
return t.hash
}

func (t *Item) GetNodeName() string {
for _, label := range nodeLabels {
if val, ok := t.Labels[label]; ok {
return string(val)
}
}

return ""
}

// NewItem Creates a new target item.
// INVARIANTS:
// * Item fields must not be modified after creation.
Expand Down