Skip to content

Commit ba9f741

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#84732 from khenidak/fix-disable-dualstack
Fix a CM panic when ipam tries to lock an out of range pre existing cidr
2 parents 78d2e52 + 2de6b2e commit ba9f741

File tree

2 files changed

+248
-0
lines changed

2 files changed

+248
-0
lines changed

‎pkg/controller/nodeipam/ipam/range_allocator.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,13 @@ func (r *rangeAllocator) occupyCIDRs(node *v1.Node) error {
228228
if err != nil {
229229
return fmt.Errorf("failed to parse node %s, CIDR %s", node.Name, node.Spec.PodCIDR)
230230
}
231+
// If node has a pre allocate cidr that does not exist in our cidrs.
232+
// This will happen if cluster went from dualstack(multi cidrs) to non-dualstack
233+
// then we have now way of locking it
234+
if idx >= len(r.cidrSets) {
235+
return fmt.Errorf("node:%s has an allocated cidr: %v at index:%v that does not exist in cluster cidrs configuration", node.Name, cidr, idx)
236+
}
237+
231238
if err := r.cidrSets[idx].Occupy(podCIDR); err != nil {
232239
return fmt.Errorf("failed to mark cidr[%v] at idx [%v] as occupied for node: %v: %v", podCIDR, idx, node.Name, err)
233240
}
@@ -284,6 +291,13 @@ func (r *rangeAllocator) ReleaseCIDR(node *v1.Node) error {
284291
return fmt.Errorf("failed to parse CIDR %s on Node %v: %v", cidr, node.Name, err)
285292
}
286293

294+
// If node has a pre allocate cidr that does not exist in our cidrs.
295+
// This will happen if cluster went from dualstack(multi cidrs) to non-dualstack
296+
// then we have now way of locking it
297+
if idx >= len(r.cidrSets) {
298+
return fmt.Errorf("node:%s has an allocated cidr: %v at index:%v that does not exist in cluster cidrs configuration", node.Name, cidr, idx)
299+
}
300+
287301
klog.V(4).Infof("release CIDR %s for node:%v", cidr, node.Name)
288302
if err = r.cidrSets[idx].Release(podCIDR); err != nil {
289303
return fmt.Errorf("error when releasing CIDR %v: %v", cidr, err)

‎pkg/controller/nodeipam/ipam/range_allocator_test.go‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,240 @@ type testCase struct {
6969
// key is index of the cidr allocated
7070
expectedAllocatedCIDR map[int]string
7171
allocatedCIDRs map[int][]string
72+
// should controller creation fail?
73+
ctrlCreateFail bool
74+
}
75+
76+
func TestOccupyPreExistingCIDR(t *testing.T) {
77+
// all tests operate on a single node
78+
testCases := []testCase{
79+
{
80+
description: "success, single stack no node allocation",
81+
fakeNodeHandler: &testutil.FakeNodeHandler{
82+
Existing: []*v1.Node{
83+
{
84+
ObjectMeta: metav1.ObjectMeta{
85+
Name: "node0",
86+
},
87+
},
88+
},
89+
Clientset: fake.NewSimpleClientset(),
90+
},
91+
clusterCIDRs: func() []*net.IPNet {
92+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
93+
return []*net.IPNet{clusterCIDRv4}
94+
}(),
95+
serviceCIDR: nil,
96+
secondaryServiceCIDR: nil,
97+
subNetMaskSize: 24,
98+
allocatedCIDRs: nil,
99+
expectedAllocatedCIDR: nil,
100+
ctrlCreateFail: false,
101+
},
102+
{
103+
description: "success, dual stack no node allocation",
104+
fakeNodeHandler: &testutil.FakeNodeHandler{
105+
Existing: []*v1.Node{
106+
{
107+
ObjectMeta: metav1.ObjectMeta{
108+
Name: "node0",
109+
},
110+
},
111+
},
112+
Clientset: fake.NewSimpleClientset(),
113+
},
114+
clusterCIDRs: func() []*net.IPNet {
115+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
116+
_, clusterCIDRv6, _ := net.ParseCIDR("ace:cab:deca::/8")
117+
return []*net.IPNet{clusterCIDRv4, clusterCIDRv6}
118+
}(),
119+
serviceCIDR: nil,
120+
secondaryServiceCIDR: nil,
121+
subNetMaskSize: 24,
122+
allocatedCIDRs: nil,
123+
expectedAllocatedCIDR: nil,
124+
ctrlCreateFail: false,
125+
},
126+
{
127+
description: "success, single stack correct node allocation",
128+
fakeNodeHandler: &testutil.FakeNodeHandler{
129+
Existing: []*v1.Node{
130+
{
131+
ObjectMeta: metav1.ObjectMeta{
132+
Name: "node0",
133+
},
134+
Spec: v1.NodeSpec{
135+
PodCIDRs: []string{"10.10.0.1/24"},
136+
},
137+
},
138+
},
139+
Clientset: fake.NewSimpleClientset(),
140+
},
141+
clusterCIDRs: func() []*net.IPNet {
142+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
143+
return []*net.IPNet{clusterCIDRv4}
144+
}(),
145+
serviceCIDR: nil,
146+
secondaryServiceCIDR: nil,
147+
subNetMaskSize: 24,
148+
allocatedCIDRs: nil,
149+
expectedAllocatedCIDR: nil,
150+
ctrlCreateFail: false,
151+
},
152+
{
153+
description: "success, dual stack both allocated correctly",
154+
fakeNodeHandler: &testutil.FakeNodeHandler{
155+
Existing: []*v1.Node{
156+
{
157+
ObjectMeta: metav1.ObjectMeta{
158+
Name: "node0",
159+
},
160+
Spec: v1.NodeSpec{
161+
PodCIDRs: []string{"10.10.0.1/24", "a00::/86"},
162+
},
163+
},
164+
},
165+
Clientset: fake.NewSimpleClientset(),
166+
},
167+
clusterCIDRs: func() []*net.IPNet {
168+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
169+
_, clusterCIDRv6, _ := net.ParseCIDR("ace:cab:deca::/8")
170+
return []*net.IPNet{clusterCIDRv4, clusterCIDRv6}
171+
}(),
172+
serviceCIDR: nil,
173+
secondaryServiceCIDR: nil,
174+
subNetMaskSize: 24,
175+
allocatedCIDRs: nil,
176+
expectedAllocatedCIDR: nil,
177+
ctrlCreateFail: false,
178+
},
179+
// failure cases
180+
{
181+
description: "fail, single stack incorrect node allocation",
182+
fakeNodeHandler: &testutil.FakeNodeHandler{
183+
Existing: []*v1.Node{
184+
{
185+
ObjectMeta: metav1.ObjectMeta{
186+
Name: "node0",
187+
},
188+
Spec: v1.NodeSpec{
189+
PodCIDRs: []string{"172.10.0.1/24"},
190+
},
191+
},
192+
},
193+
Clientset: fake.NewSimpleClientset(),
194+
},
195+
clusterCIDRs: func() []*net.IPNet {
196+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
197+
return []*net.IPNet{clusterCIDRv4}
198+
}(),
199+
serviceCIDR: nil,
200+
secondaryServiceCIDR: nil,
201+
subNetMaskSize: 24,
202+
allocatedCIDRs: nil,
203+
expectedAllocatedCIDR: nil,
204+
ctrlCreateFail: true,
205+
},
206+
{
207+
description: "fail, dualstack node allocating from non existing cidr",
208+
209+
fakeNodeHandler: &testutil.FakeNodeHandler{
210+
Existing: []*v1.Node{
211+
{
212+
ObjectMeta: metav1.ObjectMeta{
213+
Name: "node0",
214+
},
215+
Spec: v1.NodeSpec{
216+
PodCIDRs: []string{"10.10.0.1/24", "a00::/86"},
217+
},
218+
},
219+
},
220+
Clientset: fake.NewSimpleClientset(),
221+
},
222+
clusterCIDRs: func() []*net.IPNet {
223+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
224+
return []*net.IPNet{clusterCIDRv4}
225+
}(),
226+
serviceCIDR: nil,
227+
secondaryServiceCIDR: nil,
228+
subNetMaskSize: 24,
229+
allocatedCIDRs: nil,
230+
expectedAllocatedCIDR: nil,
231+
ctrlCreateFail: true,
232+
},
233+
{
234+
description: "fail, dualstack node allocating bad v4",
235+
236+
fakeNodeHandler: &testutil.FakeNodeHandler{
237+
Existing: []*v1.Node{
238+
{
239+
ObjectMeta: metav1.ObjectMeta{
240+
Name: "node0",
241+
},
242+
Spec: v1.NodeSpec{
243+
PodCIDRs: []string{"172.10.0.1/24", "a00::/86"},
244+
},
245+
},
246+
},
247+
Clientset: fake.NewSimpleClientset(),
248+
},
249+
clusterCIDRs: func() []*net.IPNet {
250+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
251+
_, clusterCIDRv6, _ := net.ParseCIDR("ace:cab:deca::/8")
252+
return []*net.IPNet{clusterCIDRv4, clusterCIDRv6}
253+
}(),
254+
serviceCIDR: nil,
255+
secondaryServiceCIDR: nil,
256+
subNetMaskSize: 24,
257+
allocatedCIDRs: nil,
258+
expectedAllocatedCIDR: nil,
259+
ctrlCreateFail: true,
260+
},
261+
{
262+
description: "fail, dualstack node allocating bad v6",
263+
264+
fakeNodeHandler: &testutil.FakeNodeHandler{
265+
Existing: []*v1.Node{
266+
{
267+
ObjectMeta: metav1.ObjectMeta{
268+
Name: "node0",
269+
},
270+
Spec: v1.NodeSpec{
271+
PodCIDRs: []string{"10.10.0.1/24", "cdd::/86"},
272+
},
273+
},
274+
},
275+
Clientset: fake.NewSimpleClientset(),
276+
},
277+
clusterCIDRs: func() []*net.IPNet {
278+
_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
279+
_, clusterCIDRv6, _ := net.ParseCIDR("ace:cab:deca::/8")
280+
return []*net.IPNet{clusterCIDRv4, clusterCIDRv6}
281+
}(),
282+
serviceCIDR: nil,
283+
secondaryServiceCIDR: nil,
284+
subNetMaskSize: 24,
285+
allocatedCIDRs: nil,
286+
expectedAllocatedCIDR: nil,
287+
ctrlCreateFail: true,
288+
},
289+
}
290+
291+
// test function
292+
for _, tc := range testCases {
293+
t.Run(tc.description, func(t *testing.T) {
294+
// Initialize the range allocator.
295+
fakeNodeInformer := getFakeNodeInformer(tc.fakeNodeHandler)
296+
nodeList, _ := tc.fakeNodeHandler.List(metav1.ListOptions{})
297+
_, err := NewCIDRRangeAllocator(tc.fakeNodeHandler, fakeNodeInformer, tc.clusterCIDRs, tc.serviceCIDR, tc.secondaryServiceCIDR, tc.subNetMaskSize, nodeList)
298+
if err == nil && tc.ctrlCreateFail {
299+
t.Fatalf("creating range allocator was expected to fail, but it did not")
300+
}
301+
if err != nil && !tc.ctrlCreateFail {
302+
t.Fatalf("creating range allocator was expected to succeed, but it did not")
303+
}
304+
})
305+
}
72306
}
73307

74308
func TestAllocateOrOccupyCIDRSuccess(t *testing.T) {

0 commit comments

Comments
 (0)