Implement test for adding new collector after completed initial alloc…

…ation (#1)

* Rename target creator helper function

* Add test for adding new collector

cmd/otel-allocator/allocation/allocatortest.go

@@ -23,14 +23,27 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target"
 )
 
+func MakeNNewTargetsWithoutPreAssigningCollectors(n int, startingIndex int) map[string]*target.Item {
+	toReturn := map[string]*target.Item{}
+	for i := startingIndex; i < n+startingIndex; i++ {
+		label := model.LabelSet{
+			"i":     model.LabelValue(strconv.Itoa(i)),
+			"total": model.LabelValue(strconv.Itoa(n + startingIndex)),
+		}
+		newTarget := target.NewItem(fmt.Sprintf("test-job-%d", i), fmt.Sprintf("test-url-%d", i), label, "")
+		toReturn[newTarget.Hash()] = newTarget
+	}
+	return toReturn
+}
+
 func colIndex(index, numCols int) int {
 	if numCols == 0 {
 		return -1
 	}
 	return index % numCols
 }
 
-func MakeNNewTargets(n int, numCollectors int, startingIndex int) map[string]*target.Item {
+func MakeNNewTargetsWithPreAssigningCollectors(n int, numCollectors int, startingIndex int) map[string]*target.Item {
 	toReturn := map[string]*target.Item{}
 	for i := startingIndex; i < n+startingIndex; i++ {
 		collector := fmt.Sprintf("collector-%d", colIndex(i, numCollectors))

cmd/otel-allocator/allocation/consistent_hashing_test.go

@@ -24,7 +24,7 @@ func TestCanSetSingleTarget(t *testing.T) {
 	cols := MakeNCollectors(3, 0)
 	c := newConsistentHashingAllocator(logger)
 	c.SetCollectors(cols)
-	c.SetTargets(MakeNNewTargets(1, 3, 0))
+	c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(1, 3, 0))
 	actualTargetItems := c.TargetItems()
 	assert.Len(t, actualTargetItems, 1)
 	for _, item := range actualTargetItems {
@@ -40,7 +40,7 @@ func TestRelativelyEvenDistribution(t *testing.T) {
 	expectedDelta := (expectedPerCollector * 1.5) - expectedPerCollector
 	c := newConsistentHashingAllocator(logger)
 	c.SetCollectors(cols)
-	c.SetTargets(MakeNNewTargets(numItems, 0, 0))
+	c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(numItems, 0, 0))
 	actualTargetItems := c.TargetItems()
 	assert.Len(t, actualTargetItems, numItems)
 	actualCollectors := c.Collectors()
@@ -54,7 +54,7 @@ func TestFullReallocation(t *testing.T) {
 	cols := MakeNCollectors(10, 0)
 	c := newConsistentHashingAllocator(logger)
 	c.SetCollectors(cols)
-	c.SetTargets(MakeNNewTargets(10000, 10, 0))
+	c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(10000, 10, 0))
 	actualTargetItems := c.TargetItems()
 	assert.Len(t, actualTargetItems, 10000)
 	actualCollectors := c.Collectors()
@@ -79,7 +79,7 @@ func TestNumRemapped(t *testing.T) {
 	cols := MakeNCollectors(numInitialCols, 0)
 	c := newConsistentHashingAllocator(logger)
 	c.SetCollectors(cols)
-	c.SetTargets(MakeNNewTargets(numItems, numInitialCols, 0))
+	c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(numItems, numInitialCols, 0))
 	actualTargetItems := c.TargetItems()
 	assert.Len(t, actualTargetItems, numItems)
 	actualCollectors := c.Collectors()

cmd/otel-allocator/allocation/least_weighted_test.go

@@ -15,6 +15,7 @@
 package allocation
 
 import (
+	"fmt"
 	"math"
 	"math/rand"
 	"testing"
@@ -50,7 +51,7 @@ func TestAddingAndRemovingTargets(t *testing.T) {
 	cols := MakeNCollectors(3, 0)
 	s.SetCollectors(cols)
 
-	initTargets := MakeNNewTargets(6, 3, 0)
+	initTargets := MakeNNewTargetsWithPreAssigningCollectors(6, 3, 0)
 
 	// test that targets and collectors are added properly
 	s.SetTargets(initTargets)
@@ -60,7 +61,7 @@ func TestAddingAndRemovingTargets(t *testing.T) {
 	assert.Len(t, s.TargetItems(), expectedTargetLen)
 
 	// prepare second round of targets
-	tar := MakeNNewTargets(4, 3, 0)
+	tar := MakeNNewTargetsWithPreAssigningCollectors(4, 3, 0)
 
 	// test that fewer targets are found - removed
 	s.SetTargets(tar)
@@ -125,7 +126,7 @@ func TestNoCollectorReassignment(t *testing.T) {
 	for _, i := range cols {
 		assert.NotNil(t, s.Collectors()[i.Name])
 	}
-	initTargets := MakeNNewTargets(6, 3, 0)
+	initTargets := MakeNNewTargetsWithPreAssigningCollectors(6, 3, 0)
 
 	// test that targets and collectors are added properly
 	s.SetTargets(initTargets)
@@ -144,6 +145,56 @@ func TestNoCollectorReassignment(t *testing.T) {
 
 }
 
+// Tests that the newly added collector instance does not get assigned any target when the targets remain the same
+func TestNoAssignmentToNewCollector(t *testing.T) {
+	s, _ := New("least-weighted", logger)
+
+	// instantiate only 1 collector
+	cols := MakeNCollectors(1, 0)
+	s.SetCollectors(cols)
+
+	expectedColLen := len(cols)
+	assert.Len(t, s.Collectors(), expectedColLen)
+
+	for _, i := range cols {
+		assert.NotNil(t, s.Collectors()[i.Name])
+	}
+
+	initialColsBeforeAddingNewCol := s.Collectors()
+	initTargets := MakeNNewTargetsWithoutPreAssigningCollectors(6, 0)
+
+	// test that targets and collectors are added properly
+	s.SetTargets(initTargets)
+
+	// verify
+	expectedTargetLen := len(initTargets)
+	targetItems := s.TargetItems()
+	assert.Len(t, targetItems, expectedTargetLen)
+
+	// add another collector
+	newColName := fmt.Sprintf("collector-%d", len(cols))
+	cols[newColName] = &Collector{
+		Name:       newColName,
+		NumTargets: 0,
+	}
+	s.SetCollectors(cols)
+
+	// targets shall not change
+	newTargetItems := s.TargetItems()
+	assert.Equal(t, targetItems, newTargetItems)
+
+	// initial collectors still should have the same targets
+	for colName, col := range s.Collectors() {
+		if colName != newColName {
+			assert.Equal(t, initialColsBeforeAddingNewCol[colName], col)
+		}
+	}
+
+	// new collector should have no targets
+	newCollector := s.Collectors()[newColName]
+	assert.Equal(t, newCollector.NumTargets, 0)
+}
+
 func TestSmartCollectorReassignment(t *testing.T) {
 	t.Skip("This test is flaky and fails frequently, see issue 1291")
 	s, _ := New("least-weighted", logger)
@@ -157,7 +208,7 @@ func TestSmartCollectorReassignment(t *testing.T) {
 	for _, i := range cols {
 		assert.NotNil(t, s.Collectors()[i.Name])
 	}
-	initTargets := MakeNNewTargets(6, 0, 0)
+	initTargets := MakeNNewTargetsWithPreAssigningCollectors(6, 0, 0)
 	// test that targets and collectors are added properly
 	s.SetTargets(initTargets)
 
@@ -202,7 +253,7 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) {
 	cols := MakeNCollectors(3, 0)
 	s.SetCollectors(cols)
 
-	targets := MakeNNewTargets(27, 3, 0)
+	targets := MakeNNewTargetsWithPreAssigningCollectors(27, 3, 0)
 	s.SetTargets(targets)
 
 	// Divisor needed to get 15%
@@ -241,7 +292,7 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) {
 		assert.InDelta(t, i.NumTargets, count, math.Round(percent))
 	}
 	// adding targets at 'random'
-	for _, item := range MakeNNewTargets(13, 3, 100) {
+	for _, item := range MakeNNewTargetsWithPreAssigningCollectors(13, 3, 100) {
 		targets[item.Hash()] = item
 	}
 	s.SetTargets(targets)

cmd/otel-allocator/allocation/strategy_test.go

@@ -44,7 +44,7 @@ func BenchmarkGetAllTargetsByCollectorAndJob(b *testing.B) {
 				b.Fail()
 			}
 			cols := MakeNCollectors(v.numCollectors, 0)
-			jobs := MakeNNewTargets(v.numJobs, v.numCollectors, 0)
+			jobs := MakeNNewTargetsWithPreAssigningCollectors(v.numJobs, v.numCollectors, 0)
 			a.SetCollectors(cols)
 			a.SetTargets(jobs)
 			b.Run(fmt.Sprintf("%s_num_cols_%d_num_jobs_%d", s, v.numCollectors, v.numJobs), func(b *testing.B) {
@@ -76,7 +76,7 @@ func Benchmark_Setting(b *testing.B) {
 		for _, v := range table {
 			a, _ := New(s, logger)
 			cols := MakeNCollectors(v.numCollectors, 0)
-			targets := MakeNNewTargets(v.numTargets, v.numCollectors, 0)
+			targets := MakeNNewTargetsWithPreAssigningCollectors(v.numTargets, v.numCollectors, 0)
 			b.Run(fmt.Sprintf("%s_num_cols_%d_num_jobs_%d", s, v.numCollectors, v.numTargets), func(b *testing.B) {
 				b.ReportAllocs()
 				for i := 0; i < b.N; i++ {

cmd/otel-allocator/server/bench_test.go

@@ -50,7 +50,7 @@ func BenchmarkServerTargetsHandler(b *testing.B) {
 		for _, v := range table {
 			a, _ := allocation.New(allocatorName, logger)
 			cols := allocation.MakeNCollectors(v.numCollectors, 0)
-			targets := allocation.MakeNNewTargets(v.numJobs, v.numCollectors, 0)
+			targets := allocation.MakeNNewTargetsWithPreAssigningCollectors(v.numJobs, v.numCollectors, 0)
 			listenAddr := ":8080"
 			a.SetCollectors(cols)
 			a.SetTargets(targets)