Skip to content

Commit 3b558e3

Browse files
committed
fix: skip IPAM init for empty IP annotation and handle nil IPRangeList (kubeovn#6296)
- Skip IPAM initialization when pod IP annotation is empty, log warning instead - Make IPRangeList.Len() nil-safe to prevent panic in GetSubnetIPRangeString - Add unit test for nil IPRangeList.Separate() behavior Signed-off-by: zbb88888 <jmdxjsjgcxy@gmail.com>
1 parent d0963e4 commit 3b558e3

File tree

3 files changed

+118
-4
lines changed

3 files changed

+118
-4
lines changed

pkg/controller/init.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,9 +452,13 @@ func (c *Controller) InitIPAM() error {
452452
portName := ovs.PodNameToPortName(podName, pod.Namespace, podNet.ProviderName)
453453
ip := pod.Annotations[fmt.Sprintf(util.IPAddressAnnotationTemplate, podNet.ProviderName)]
454454
mac := pod.Annotations[fmt.Sprintf(util.MacAddressAnnotationTemplate, podNet.ProviderName)]
455+
if ip == "" {
456+
klog.Warningf("pod %s/%s has empty IP annotation for provider %s, skip IPAM init", pod.Namespace, podName, podNet.ProviderName)
457+
continue
458+
}
455459
_, _, _, err := c.ipam.GetStaticAddress(key, portName, ip, &mac, podNet.Subnet.Name, true)
456460
if err != nil {
457-
klog.Errorf("failed to init pod %s.%s address %s: %v", podName, pod.Namespace, pod.Annotations[fmt.Sprintf(util.IPAddressAnnotationTemplate, podNet.ProviderName)], err)
461+
klog.Errorf("failed to init pod %s.%s address %s: %v", podName, pod.Namespace, ip, err)
458462
} else {
459463
err = c.createOrUpdateIPCR(portName, podName, ip, mac, podNet.Subnet.Name, pod.Namespace, pod.Spec.NodeName, podType)
460464
if err != nil {

pkg/ipam/ip_range_list.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ func NewIPRangeListFrom(x ...string) (*IPRangeList, error) {
7777
}
7878

7979
func (r *IPRangeList) Clone() *IPRangeList {
80+
if r == nil {
81+
return NewEmptyIPRangeList()
82+
}
8083
ret := &IPRangeList{make([]*IPRange, r.Len())}
8184
for i := range r.ranges {
8285
ret.ranges[i] = r.ranges[i].Clone()
@@ -85,10 +88,16 @@ func (r *IPRangeList) Clone() *IPRangeList {
8588
}
8689

8790
func (r *IPRangeList) Len() int {
91+
if r == nil {
92+
return 0
93+
}
8894
return len(r.ranges)
8995
}
9096

9197
func (r *IPRangeList) Count() internal.BigInt {
98+
if r == nil {
99+
return internal.BigInt{}
100+
}
92101
var count internal.BigInt
93102
for _, v := range r.ranges {
94103
count = count.Add(v.Count())
@@ -97,13 +106,16 @@ func (r *IPRangeList) Count() internal.BigInt {
97106
}
98107

99108
func (r *IPRangeList) At(i int) *IPRange {
100-
if i < len(r.ranges) {
101-
return r.ranges[i]
109+
if r == nil || i < 0 || i >= len(r.ranges) {
110+
return nil
102111
}
103-
return nil
112+
return r.ranges[i]
104113
}
105114

106115
func (r *IPRangeList) Find(ip IP) (int, bool) {
116+
if r == nil {
117+
return 0, false
118+
}
107119
return sort.Find(len(r.ranges), func(i int) int {
108120
if r.At(i).Start().GreaterThan(ip) {
109121
return -1
@@ -121,6 +133,9 @@ func (r *IPRangeList) Contains(ip IP) bool {
121133
}
122134

123135
func (r *IPRangeList) Add(ip IP) bool {
136+
if r == nil {
137+
return false
138+
}
124139
n, ok := r.Find(ip)
125140
if ok {
126141
return false
@@ -145,6 +160,9 @@ func (r *IPRangeList) Add(ip IP) bool {
145160
}
146161

147162
func (r *IPRangeList) Remove(ip IP) bool {
163+
if r == nil {
164+
return false
165+
}
148166
n, ok := r.Find(ip)
149167
if !ok {
150168
return false

pkg/ipam/ip_range_list_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,3 +1388,95 @@ func TestIPRangeListToCIDRsEdgeCases(t *testing.T) {
13881388
require.Equal(t, []string{"::/128"}, result)
13891389
})
13901390
}
1391+
1392+
func TestIPRangeList_NilReceiverSafety(t *testing.T) {
1393+
// Background: In single-stack subnets, V4Available or V6Available can be nil.
1394+
// All IPRangeList methods must handle nil receiver without panic.
1395+
var nilList *IPRangeList
1396+
ip, _ := NewIP("10.0.0.1")
1397+
1398+
t.Run("Len", func(t *testing.T) {
1399+
require.NotPanics(t, func() {
1400+
require.Equal(t, 0, nilList.Len())
1401+
})
1402+
})
1403+
1404+
t.Run("Count", func(t *testing.T) {
1405+
require.NotPanics(t, func() {
1406+
require.Equal(t, "0", nilList.Count().String())
1407+
})
1408+
})
1409+
1410+
t.Run("Clone", func(t *testing.T) {
1411+
var result *IPRangeList
1412+
require.NotPanics(t, func() {
1413+
result = nilList.Clone()
1414+
})
1415+
require.NotNil(t, result)
1416+
require.Equal(t, 0, result.Len())
1417+
})
1418+
1419+
t.Run("At", func(t *testing.T) {
1420+
require.NotPanics(t, func() {
1421+
require.Nil(t, nilList.At(0))
1422+
})
1423+
})
1424+
1425+
t.Run("Find", func(t *testing.T) {
1426+
require.NotPanics(t, func() {
1427+
idx, found := nilList.Find(ip)
1428+
require.Equal(t, 0, idx)
1429+
require.False(t, found)
1430+
})
1431+
})
1432+
1433+
t.Run("Contains", func(t *testing.T) {
1434+
require.NotPanics(t, func() {
1435+
require.False(t, nilList.Contains(ip))
1436+
})
1437+
})
1438+
1439+
t.Run("Add", func(t *testing.T) {
1440+
require.NotPanics(t, func() {
1441+
require.False(t, nilList.Add(ip))
1442+
})
1443+
})
1444+
1445+
t.Run("Remove", func(t *testing.T) {
1446+
require.NotPanics(t, func() {
1447+
require.False(t, nilList.Remove(ip))
1448+
})
1449+
})
1450+
1451+
t.Run("Separate", func(t *testing.T) {
1452+
var result *IPRangeList
1453+
require.NotPanics(t, func() {
1454+
result = nilList.Separate(nil)
1455+
})
1456+
require.NotNil(t, result)
1457+
require.Equal(t, 0, result.Len())
1458+
})
1459+
1460+
t.Run("String", func(t *testing.T) {
1461+
require.NotPanics(t, func() {
1462+
require.Equal(t, "", nilList.String())
1463+
})
1464+
})
1465+
1466+
t.Run("ToCIDRs", func(t *testing.T) {
1467+
require.NotPanics(t, func() {
1468+
cidrs, err := nilList.ToCIDRs()
1469+
require.NoError(t, err)
1470+
require.Nil(t, cidrs)
1471+
})
1472+
})
1473+
}
1474+
1475+
func TestIPRangeListSeparate_WithEmptyList(t *testing.T) {
1476+
// An empty but non-nil list returns empty result immediately without using 'other'
1477+
emptyList := NewEmptyIPRangeList()
1478+
1479+
result := emptyList.Separate(nil)
1480+
require.NotNil(t, result)
1481+
require.Equal(t, 0, result.Len())
1482+
}

0 commit comments

Comments
 (0)