From 15fac8ab96151871a461498d65e932cb0b611399 Mon Sep 17 00:00:00 2001
From: Takashi Kusumi <tkusumi@zlab.co.jp>
Date: Tue, 10 May 2022 15:55:42 +0900
Subject: [PATCH] Fix ServiceIPStaticSubrange assigns duplicate IP addresses

---
 pkg/registry/core/service/allocator/bitmap.go |  2 +-
 .../core/service/allocator/bitmap_test.go     | 24 ++++++++++++++
 .../ipallocator/storage/storage_test.go       | 31 +++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/pkg/registry/core/service/allocator/bitmap.go b/pkg/registry/core/service/allocator/bitmap.go
index 8cf1377bd2c..b492b4a1bb6 100644
--- a/pkg/registry/core/service/allocator/bitmap.go
+++ b/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
diff --git a/pkg/registry/core/service/allocator/bitmap_test.go b/pkg/registry/core/service/allocator/bitmap_test.go
index f9af6a1aebc..586f5703f24 100644
--- a/pkg/registry/core/service/allocator/bitmap_test.go
+++ b/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))
+	}
+}
diff --git a/pkg/registry/core/service/ipallocator/storage/storage_test.go b/pkg/registry/core/service/ipallocator/storage/storage_test.go
index 706e7bc48b6..f39b5012990 100644
--- a/pkg/registry/core/service/ipallocator/storage/storage_test.go
+++ b/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")
+	}
+}