Merge pull request #109928 from zlabjp/fix-reserved-allocator-dup

Fix ServiceIPStaticSubrange assigns duplicate IP addresses

pkg/registry/core/service/allocator/bitmap.go

@@ -254,7 +254,7 @@ func (rss randomScanStrategyWithOffset) AllocateBit(allocated *big.Int, max, cou
 	for i := 0; i < rss.offset; i++ {
 		at := (start + i) % rss.offset
 		if allocated.Bit(at) == 0 {
-			return i, true
+			return at, true
 		}
 	}
 	return 0, false

pkg/registry/core/service/allocator/bitmap_test.go

@@ -541,3 +541,27 @@ func TestPreAllocateReservedFull_BitmapReserved(t *testing.T) {
 		t.Errorf("expect to get %d, but got %d", max-1, f)
 	}
 }
+
+func TestAllocateUniqueness(t *testing.T) {
+	max := 128
+	dynamicOffset := 16
+	uniqueAllocated := map[int]bool{}
+	m := NewAllocationMapWithOffset(max, "test", dynamicOffset)
+
+	// Allocate all the values in both the dynamic and reserved blocks
+	for i := 0; i < max; i++ {
+		alloc, ok, _ := m.AllocateNext()
+		if !ok {
+			t.Fatalf("unexpected error")
+		}
+		if _, ok := uniqueAllocated[alloc]; ok {
+			t.Fatalf("unexpected allocated value %d", alloc)
+		} else {
+			uniqueAllocated[alloc] = true
+		}
+	}
+
+	if max != len(uniqueAllocated) {
+		t.Errorf("expect to get %d, but got %d", max, len(uniqueAllocated))
+	}
+}

pkg/registry/core/service/ipallocator/storage/storage_test.go

@@ -167,3 +167,34 @@ func TestAllocateReserved(t *testing.T) {
 		t.Error("Allocator expected to be full")
 	}
 }
+
+func TestAllocateReservedDynamicBlockExhausted(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServiceIPStaticSubrange, true)()
+
+	_, storage, _, si, destroyFunc := newStorage(t)
+	defer destroyFunc()
+	if err := si.Create(context.TODO(), key(), validNewRangeAllocation(), nil, 0); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// allocate all addresses both on the dynamic and reserved blocks
+	// once the dynamic block has been exhausted
+	// the dynamic allocator will use the reserved block
+	max := 254
+
+	for i := 0; i < max; i++ {
+		if _, err := storage.AllocateNext(); err != nil {
+			t.Errorf("Unexpected error trying to allocate: %v", err)
+		}
+	}
+	for i := 0; i < max; i++ {
+		ip := fmt.Sprintf("192.168.1.%d", i+1)
+		if !storage.Has(netutils.ParseIPSloppy(ip)) {
+			t.Errorf("IP %s expected to be allocated", ip)
+		}
+	}
+
+	if _, err := storage.AllocateNext(); err == nil {
+		t.Error("Allocator expected to be full")
+	}
+}